mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 08:20:43 +00:00
fix(model-selection): address Codex review P2 feedback on auto-heal override clearing
Three corrections to the auto-failover self-healing introduced in the prior commit: 1. Reset in-memory provider/model to configured primary after clearing auto override. get-reply-directives.ts preloads provider/model from the stored override before calling createModelSelectionState, so clearing only session state still ran the current turn on the fallback. Now provider/model are reset to defaultProvider/ defaultModel so this turn retries the primary immediately, not on the next turn. 2. Remove resetModelOverride = true from the auto-heal path. That flag triggers a "Model override not allowed for this agent" system event in applyInlineDirectiveOverrides, which is incorrect: the override was valid and set by the fallback loop — it just expired once the primary recovered. Auto-heal is not an allowlist violation. 3. Add a test case that verifies the in-memory reset when the caller pre-loads the fallback provider/model (simulating the get-reply-directives.ts preload path). Known limitation (noted in comment): channel model overrides (channels.modelByChannel) are skipped on the recovery turn because hasSessionModelOverride was true when they were evaluated at preload time. They resume on the following turn once session state is clear. Fixing this cleanly requires changes to the get-reply-directives preload flow and is out of scope for this PR.
This commit is contained in:
committed by
Peter Steinberger
parent
f2abe28d40
commit
dc0e966ed2
@@ -568,6 +568,7 @@ describe("createModelSelectionState auto-failover override self-healing", () =>
|
||||
modelOverrideSource: "auto",
|
||||
});
|
||||
|
||||
// Provider/model should revert to the configured primary, not the fallback.
|
||||
// Provider/model should revert to the configured primary, not the fallback.
|
||||
expect(state.provider).toBe(defaultProvider);
|
||||
expect(state.model).toBe(defaultModel);
|
||||
@@ -575,7 +576,39 @@ describe("createModelSelectionState auto-failover override self-healing", () =>
|
||||
expect(sessionStore[sessionKey]?.providerOverride).toBeUndefined();
|
||||
expect(sessionStore[sessionKey]?.modelOverride).toBeUndefined();
|
||||
expect(sessionStore[sessionKey]?.modelOverrideSource).toBeUndefined();
|
||||
expect(state.resetModelOverride).toBe(true);
|
||||
// resetModelOverride must NOT be set — it triggers a "Model override not allowed"
|
||||
// system event which is incorrect for auto-heal (the override was valid).
|
||||
expect(state.resetModelOverride).toBe(false);
|
||||
});
|
||||
|
||||
it("resets in-memory provider/model even when caller pre-loaded the fallback", async () => {
|
||||
// Simulates get-reply-directives.ts preloading provider/model from stored override
|
||||
// before calling createModelSelectionState. Our fix must update those in-memory
|
||||
// values so the current turn retries the primary, not the fallback.
|
||||
const cfg = {} as OpenClawConfig;
|
||||
const sessionEntry = makeEntry({
|
||||
providerOverride: "openrouter",
|
||||
modelOverride: "minimax/minimax-m2.7",
|
||||
modelOverrideSource: "auto",
|
||||
});
|
||||
const sessionStore = { [sessionKey]: sessionEntry };
|
||||
const state = await createModelSelectionState({
|
||||
cfg,
|
||||
agentCfg: cfg.agents?.defaults,
|
||||
sessionEntry,
|
||||
sessionStore,
|
||||
sessionKey,
|
||||
defaultProvider,
|
||||
defaultModel,
|
||||
// Caller already preloaded fallback values from stored override
|
||||
provider: "openrouter",
|
||||
model: "minimax/minimax-m2.7",
|
||||
hasModelDirective: false,
|
||||
});
|
||||
|
||||
expect(state.provider).toBe(defaultProvider);
|
||||
expect(state.model).toBe(defaultModel);
|
||||
expect(state.resetModelOverride).toBe(false);
|
||||
});
|
||||
|
||||
it("preserves a user-selected override across turns", async () => {
|
||||
|
||||
@@ -376,10 +376,14 @@ export async function createModelSelectionState(params: {
|
||||
// the regular session/parent model override behavior.
|
||||
const skipStoredOverride = params.hasResolvedHeartbeatModelOverride === true;
|
||||
|
||||
// Auto-failover overrides are transient: on the next turn, retry the configured
|
||||
// Auto-failover overrides are transient: on this turn, retry the configured
|
||||
// primary so the session self-heals when the primary recovers. The fallback loop
|
||||
// in runWithModelFallback will re-set the override if the primary is still down.
|
||||
// User-selected overrides (/model command) are preserved across turns.
|
||||
//
|
||||
// Note: channel model overrides (channels.modelByChannel) are skipped when
|
||||
// hasSessionModelOverride was true at get-reply-directives preload time. They
|
||||
// resume on the following turn once the session state is clear.
|
||||
const isAutoSessionOverride =
|
||||
storedOverride?.source === "session" && sessionEntry?.modelOverrideSource === "auto";
|
||||
if (isAutoSessionOverride && sessionEntry && sessionStore && sessionKey && !resetModelOverride) {
|
||||
@@ -396,7 +400,15 @@ export async function createModelSelectionState(params: {
|
||||
store[sessionKey] = sessionEntry;
|
||||
});
|
||||
}
|
||||
resetModelOverride = true;
|
||||
// Reset in-memory selection to the configured primary. The caller-provided
|
||||
// provider/model were already set to the fallback by the stored-override
|
||||
// preload in get-reply-directives.ts; updating them here ensures this turn
|
||||
// retries the primary rather than incurring one extra fallback call.
|
||||
provider = defaultProvider;
|
||||
model = defaultModel;
|
||||
// Do NOT set resetModelOverride — that flag triggers a "Model override not
|
||||
// allowed for this agent" system event, which is incorrect for auto-heal.
|
||||
// The override was valid; it just expired after the primary recovered.
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user