fix(agent): re-check context on cooldown-expiry revert in auto-continue path
A cooldown-expiry model revert runs at a turn boundary. The user-prompt path reverts then re-checks accumulated context against the restored model via runPrePromptCompactionIfNeeded, but the automatic agent.continue() path (#scheduleAgentContinue) reverted and issued the next request with no such check. When a transient failure had fallen 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. maybeRestoreRetryFallbackPrimary now reports whether it actually switched, and the auto-continue path runs the same post-revert context-fit maintenance (compaction/promotion) the prompt path already runs, but only when a revert occurred. Fixes #7952
This commit is contained in:
@@ -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://<name>?op=...` commands targeting the focused/most-recently-active vault instead of the named one. The `vault=<name>` 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
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -308,8 +308,13 @@ export class TurnRecovery {
|
||||
return this.#runRecoveryCompactionWithRollback(reason, message, allowDefer, options);
|
||||
}
|
||||
|
||||
/** Restores the configured primary after fallback cooldown expiry. */
|
||||
maybeRestoreRetryFallbackPrimary(): Promise<void> {
|
||||
/**
|
||||
* 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<boolean> {
|
||||
return this.#maybeRestoreRetryFallbackPrimary();
|
||||
}
|
||||
|
||||
@@ -1447,10 +1452,10 @@ export class TurnRecovery {
|
||||
return true;
|
||||
}
|
||||
|
||||
async #maybeRestoreRetryFallbackPrimary(): Promise<void> {
|
||||
if (!this.#activeRetryFallback) return;
|
||||
if (this.#activeRetryFallback.pinned) return;
|
||||
if (this.#getRetryFallbackRevertPolicy() !== "cooldown-expiry") return;
|
||||
async #maybeRestoreRetryFallbackPrimary(): Promise<boolean> {
|
||||
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 {
|
||||
|
||||
@@ -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");
|
||||
|
||||
Reference in New Issue
Block a user