diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index f0d381585..7f54280d4 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -36,6 +36,7 @@ - Fixed ACP `session/load` and `session/resume` failing with `ACP session not found` for sessions created under the legacy/hashed project-directory scheme (17.2.5+, reverted in #7656): the lookup only scanned the directory re-derived from `cwd`, so sessions stored under a differently-named directory were unreachable. It now falls back to a global by-id scan (the same one the fork path already uses) when the cwd-scoped lookup misses ([#7779](https://github.com/can1357/oh-my-pi/issues/7779)). - Fixed `vault://?op=...` commands targeting the focused/most-recently-active vault instead of the named one. The `vault=` argument was appended after the Obsidian CLI subcommand (`obsidian bases vault=Work`), but the CLI only honors it as a top-level option before the subcommand (`obsidian vault=Work bases`); it is now prepended so the named vault is queried (and opened) regardless of window focus ([#7771](https://github.com/can1357/oh-my-pi/issues/7771)). - The status-line `session_name` segment now honors the `statusLine.sessionAccent` setting: when disabled, the rendered session name falls back to the theme `accent` color instead of emitting the hash-derived session accent, matching the gap-fill divider behavior ([#7867](https://github.com/can1357/oh-my-pi/pull/7867)). +- Fixed a cooldown-expiry model revert reverting onto a smaller-context model without re-checking the accumulated context in the automatic `agent.continue()` path. When a transient failure fell back to a larger-window model and the conversation then grew past the original model's window, restoring the primary once its cooldown expired sent a predictably oversized request to the smaller model (no compaction, promotion, or warning) rather than the pre-send context check the user-prompt path already ran. The auto-continue path now runs the same context-fit maintenance after a revert ([#7952](https://github.com/can1357/oh-my-pi/issues/7952)). ### Fixed diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 6c5df21d7..707e87a5b 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -3020,11 +3020,23 @@ export class AgentSession { } this.#beginInFlight(); try { - await this.#recovery.maybeRestoreRetryFallbackPrimary(); + const reverted = await this.#recovery.maybeRestoreRetryFallbackPrimary(); if (signal.aborted || this.#isDisposed) { this.#skipAgentContinue("post-restore-unavailable", options); return; } + // A cooldown-expiry revert can drop the active window below the + // accumulated context. The user-prompt path re-checks context after + // the revert via runPrePromptCompactionIfNeeded; the auto-continue + // path must do the same so agent.continue() never sends a + // predictably oversized request to the reverted (smaller) model. + if (reverted) { + await this.#maintenance.runPrePromptCompactionIfNeeded([]); + if (signal.aborted || this.#isDisposed) { + this.#skipAgentContinue("post-restore-unavailable", options); + return; + } + } if (this.settings.get("retry.usageAwareFallback")) { if (!(await this.#runQueuedUsageAwarePreflight(signal))) { this.#skipAgentContinue("session-unavailable", options); diff --git a/packages/coding-agent/src/session/turn-recovery.ts b/packages/coding-agent/src/session/turn-recovery.ts index b28f212f0..69078e959 100644 --- a/packages/coding-agent/src/session/turn-recovery.ts +++ b/packages/coding-agent/src/session/turn-recovery.ts @@ -308,8 +308,13 @@ export class TurnRecovery { return this.#runRecoveryCompactionWithRollback(reason, message, allowDefer, options); } - /** Restores the configured primary after fallback cooldown expiry. */ - maybeRestoreRetryFallbackPrimary(): Promise { + /** + * Restores the configured primary after fallback cooldown expiry. + * @returns true when the active model was actually switched back to the + * primary, so callers can re-run the pre-send context-fit check against the + * reverted (possibly smaller) window before issuing the next request. + */ + maybeRestoreRetryFallbackPrimary(): Promise { return this.#maybeRestoreRetryFallbackPrimary(); } @@ -1447,10 +1452,10 @@ export class TurnRecovery { return true; } - async #maybeRestoreRetryFallbackPrimary(): Promise { - if (!this.#activeRetryFallback) return; - if (this.#activeRetryFallback.pinned) return; - if (this.#getRetryFallbackRevertPolicy() !== "cooldown-expiry") return; + async #maybeRestoreRetryFallbackPrimary(): Promise { + if (!this.#activeRetryFallback) return false; + if (this.#activeRetryFallback.pinned) return false; + if (this.#getRetryFallbackRevertPolicy() !== "cooldown-expiry") return false; const { originalSelector: originalSelectorRaw, @@ -1460,19 +1465,19 @@ export class TurnRecovery { const originalSelector = parseRetryFallbackSelector(originalSelectorRaw, this.#host.modelRegistry); if (!originalSelector) { this.clearActiveRetryFallback(); - return; + return false; } const currentModel = this.#host.model(); - if (!currentModel) return; + if (!currentModel) return false; const currentSelector = formatRetryFallbackSelector(currentModel, this.#host.thinkingLevel()); if (currentSelector === originalSelector.raw) { if (!this.isRetryFallbackSelectorSuppressed(originalSelector)) { this.clearActiveRetryFallback(); } - return; + return false; } - if (this.isRetryFallbackSelectorSuppressed(originalSelector)) return; + if (this.isRetryFallbackSelectorSuppressed(originalSelector)) return false; const resolvedPrimary = resolveModelOverride( [originalSelector.raw], @@ -1481,9 +1486,9 @@ export class TurnRecovery { ); const primaryModel = resolvedPrimary.model ?? this.#host.modelRegistry.find(originalSelector.provider, originalSelector.id); - if (!primaryModel) return; + if (!primaryModel) return false; const apiKey = await this.#host.modelRegistry.getApiKey(primaryModel, this.#host.sessionId()); - if (!apiKey) return; + if (!apiKey) return false; const currentThinkingLevel = this.#host.configuredThinkingLevel(); const thinkingToApply = @@ -1494,6 +1499,7 @@ export class TurnRecovery { this.#host.settings.getStorage()?.recordModelUsage(primarySelector); this.#host.setThinkingLevel(thinkingToApply); this.clearActiveRetryFallback(); + return true; } #parseRetryAfterMsFromError(errorMessage: string): number | undefined { 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 9b471e032..1dcfa977d 100644 --- a/packages/coding-agent/test/agent-session-retry-fallback.test.ts +++ b/packages/coding-agent/test/agent-session-retry-fallback.test.ts @@ -3395,6 +3395,120 @@ describe("AgentSession retry fallback", () => { expect(session.model?.id).toBe(primaryModel.id); }); + it("re-checks context before a cooldown-expiry revert onto a smaller-window model in the auto-continue path", async () => { + // Regression for #7952: a cooldown-expiry revert reverts the model at a + // turn boundary. The user-prompt path re-checks context after the revert + // (runPrePromptCompactionIfNeeded), but the automatic agent.continue() + // path did not — so reverting onto a model whose window is smaller than + // the accumulated context sent a predictably oversized request. Here the + // small primary (4000-token window) fell back to a large-window model, + // accumulated context past 4000 while there, then the cooldown expired and + // a queued follow-up drained through the auto-continue path. + const modelsConfigPath = path.join(tempDir.path(), "revert-overflow-models.json"); + await Bun.write( + modelsConfigPath, + JSON.stringify({ + providers: { + 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("openai", "gpt-4o-mini"); + const fallbackModel = modelRegistry.find("openai", "gpt-4o"); + if (!primaryModel || !fallbackModel) { + throw new Error("Expected override models to resolve"); + } + expect(primaryModel.contextWindow).toBe(4000); + expect(fallbackModel.contextWindow).toBe(1_000_000); + + // ~15k estimated tokens: over the primary's 4000 window (80% => 3200) but + // far under the fallback's (800k), so it sits on the fallback without + // compaction and only overflows once the window shrinks on revert. + const bigText = "lorem ipsum ".repeat(5000); + const requestedModels: string[] = []; + const mock = createMockModel(); + let primaryAttempts = 0; + let fallbackTurns = 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 if (model.id === fallbackModel.id && fallbackTurns === 0) { + fallbackTurns += 1; + mock.push({ content: [bigText] }); + } 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: [`${fallbackModel.provider}/${fallbackModel.id}`], + }, + "retry.fallbackRevertPolicy": "cooldown-expiry", + }); + settings.setModelRole("default", `${primaryModel.provider}/${primaryModel.id}`); + + session = new AgentSession({ + agent, + sessionManager: SessionManager.inMemory(), + settings, + modelRegistry, + }); + let now = Date.now(); + vi.spyOn(Date, "now").mockImplementation(() => now); + + // Primary rate-limits, falls back to the large-window model, and that turn + // returns a large payload that grows context past the primary's window. + await session.prompt("Trigger fallback and grow context past the primary window"); + await session.waitForIdle(); + expect(requestedModels).toEqual([ + `${primaryModel.provider}/${primaryModel.id}`, + `${fallbackModel.provider}/${fallbackModel.id}`, + ]); + expect(session.model?.id).toBe(fallbackModel.id); + + // Cooldown expires; a queued follow-up drains through the auto-continue + // (agent.continue) path, which reverts to the primary. The post-revert + // context check now runs there too: the accumulated context no longer fits + // the primary's 4000 window, so it promotes to the larger-window model + // instead of issuing the oversized request. Before the fix the session + // stayed on the reverted primary and received the over-window request. + now += 60_000; + await session.followUp("Please continue on the reverted primary"); + await session.waitForIdle(); + + expect(session.model?.id).toBe(fallbackModel.id); + expect(requestedModels.at(-1)).toBe(`${fallbackModel.provider}/${fallbackModel.id}`); + // The 4000-window primary is only ever hit by the initial rate-limited + // request — never by an over-window continuation after the revert. + expect(requestedModels.filter(id => id === `${primaryModel.provider}/${primaryModel.id}`)).toHaveLength(1); + }); + 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");