From 23f94bfa785e196cf3e3b2db848fa8aedcf7312b Mon Sep 17 00:00:00 2001 From: snowzlmbot Date: Tue, 23 Jun 2026 03:14:25 +0800 Subject: [PATCH] fix(reply): normalize persisted model overrides before reset (#94752) Co-authored-by: snowzlm --- .../reply/get-reply-directives-apply.test.ts | 12 +++ .../reply/get-reply-directives-apply.ts | 8 ++ src/auto-reply/reply/model-selection.test.ts | 83 +++++++++++++++++++ src/auto-reply/reply/model-selection.ts | 14 +++- src/auto-reply/reply/stored-model-override.ts | 31 +++++-- 5 files changed, 139 insertions(+), 9 deletions(-) diff --git a/src/auto-reply/reply/get-reply-directives-apply.test.ts b/src/auto-reply/reply/get-reply-directives-apply.test.ts index 89ab8324e868..0ecc553ae296 100644 --- a/src/auto-reply/reply/get-reply-directives-apply.test.ts +++ b/src/auto-reply/reply/get-reply-directives-apply.test.ts @@ -21,4 +21,16 @@ describe("formatModelOverrideResetEvent", () => { }), ).toBe("Model override not allowed for this agent; reverted to github-copilot/gpt-4o."); }); + + it("does not tell users to edit the allowlist for stale session overrides", () => { + expect( + formatModelOverrideResetEvent({ + rejectedRef: "openai/gpt-5.5", + initialModelLabel: "openai/gpt-5.4", + reason: "stale", + }), + ).toBe( + "Stored model override openai/gpt-5.5 is stale for this session; reverted to openai/gpt-5.4. Pick a model again with /model if you still want to override the default.", + ); + }); }); diff --git a/src/auto-reply/reply/get-reply-directives-apply.ts b/src/auto-reply/reply/get-reply-directives-apply.ts index 9f2d05c46c91..239cd0665b3c 100644 --- a/src/auto-reply/reply/get-reply-directives-apply.ts +++ b/src/auto-reply/reply/get-reply-directives-apply.ts @@ -68,7 +68,14 @@ function hasOnlyModelDirective(directives: InlineDirectives): boolean { export function formatModelOverrideResetEvent(params: { rejectedRef?: string; initialModelLabel: string; + reason?: "disallowed" | "stale"; }): string { + if (params.reason === "stale") { + if (params.rejectedRef) { + return `Stored model override ${params.rejectedRef} is stale for this session; reverted to ${params.initialModelLabel}. Pick a model again with /model if you still want to override the default.`; + } + return `Stored model override is stale for this session; reverted to ${params.initialModelLabel}.`; + } if (params.rejectedRef) { return `Model override ${params.rejectedRef} is not allowed for this agent; reverted to ${params.initialModelLabel}. Add ${params.rejectedRef} to agents.defaults.models or pick an allowed model with /model list.`; } @@ -194,6 +201,7 @@ export async function applyInlineDirectiveOverrides(params: { formatModelOverrideResetEvent({ rejectedRef: modelState.resetModelOverrideRef, initialModelLabel, + reason: modelState.resetModelOverrideReason, }), { sessionKey, diff --git a/src/auto-reply/reply/model-selection.test.ts b/src/auto-reply/reply/model-selection.test.ts index 40fc2185be5c..32a6fca8a282 100644 --- a/src/auto-reply/reply/model-selection.test.ts +++ b/src/auto-reply/reply/model-selection.test.ts @@ -1025,6 +1025,89 @@ describe("createModelSelectionState respects session model override", () => { expect(state.resetModelOverride).toBe(false); }); + it("keeps provider-qualified stored overrides when providerOverride is also persisted", async () => { + const cfg = { + agents: { + defaults: { + model: { primary: "openai/gpt-5.5" }, + models: { + "openai/gpt-5.5": {}, + "openai/gpt-5.4": {}, + }, + }, + }, + } as OpenClawConfig; + const sessionKey = "agent:main:dashboard:child"; + const sessionEntry = makeEntry({ + providerOverride: "openai", + modelOverride: "openai/gpt-5.5", + modelOverrideSource: "user", + }); + const sessionStore = { [sessionKey]: sessionEntry }; + + const state = await createModelSelectionState({ + cfg, + agentCfg: cfg.agents?.defaults, + sessionEntry, + sessionStore, + sessionKey, + defaultProvider: "openai", + defaultModel: "gpt-5.5", + provider: "openai", + model: "gpt-5.4", + hasModelDirective: false, + }); + + expect(state.provider).toBe("openai"); + expect(state.model).toBe("gpt-5.5"); + expect(state.resetModelOverride).toBe(false); + expect(sessionStore[sessionKey]?.providerOverride).toBe("openai"); + expect(sessionStore[sessionKey]?.modelOverride).toBe("openai/gpt-5.5"); + }); + + it("normalizes provider-qualified parent stored overrides before allowlist checks", async () => { + const cfg = { + agents: { + defaults: { + model: { primary: "openai/gpt-5.5" }, + models: { + "openai/gpt-5.5": {}, + "openai/gpt-5.4": {}, + }, + }, + }, + } as OpenClawConfig; + const parentSessionKey = "agent:main:dashboard:parent"; + const sessionKey = "agent:main:dashboard:child"; + const sessionEntry = makeEntry(); + const parentEntry = makeEntry({ + providerOverride: "openai", + modelOverride: "openai/gpt-5.5", + modelOverrideSource: "user", + }); + const sessionStore = { [sessionKey]: sessionEntry, [parentSessionKey]: parentEntry }; + + const state = await createModelSelectionState({ + cfg, + agentCfg: cfg.agents?.defaults, + sessionEntry, + sessionStore, + sessionKey, + parentSessionKey, + defaultProvider: "openai", + defaultModel: "gpt-5.5", + provider: "openai", + model: "gpt-5.4", + hasModelDirective: false, + }); + + expect(state.provider).toBe("openai"); + expect(state.model).toBe("gpt-5.5"); + expect(state.resetModelOverride).toBe(false); + expect(sessionStore[parentSessionKey]?.modelOverride).toBe("openai/gpt-5.5"); + expect(sessionStore[sessionKey]?.modelOverride).toBeUndefined(); + }); + it("clears disallowed model overrides and falls back to the default", async () => { const cfg = { agents: { diff --git a/src/auto-reply/reply/model-selection.ts b/src/auto-reply/reply/model-selection.ts index be7e8a0b0ac5..e6c637dd42c1 100644 --- a/src/auto-reply/reply/model-selection.ts +++ b/src/auto-reply/reply/model-selection.ts @@ -16,6 +16,7 @@ import { modelKey, normalizeModelRef, normalizeProviderId, + normalizeStoredOverrideModel, resolvePersistedOverrideModelRef, resolveReasoningDefault, resolveThinkingDefault, @@ -53,6 +54,7 @@ type ModelSelectionState = { allowedModelCatalog: ModelCatalog; resetModelOverride: boolean; resetModelOverrideRef?: string; + resetModelOverrideReason?: "disallowed" | "stale"; resolveThinkingCatalog: () => Promise; resolveDefaultThinkingLevel: () => Promise; /** Default reasoning level from model capability: "on" if model has reasoning, else "off". */ @@ -75,6 +77,7 @@ export function createFastTestModelSelectionState(params: { allowedModelCatalog: [], resetModelOverride: false, resetModelOverrideRef: undefined, + resetModelOverrideReason: undefined, resolveThinkingCatalog: async () => [], resolveDefaultThinkingLevel: async () => params.agentCfg?.thinkingDefault as ThinkLevel, resolveDefaultReasoningLevel: async () => "off", @@ -194,11 +197,16 @@ export async function createModelSelectionState(params: { let modelCatalog: ModelCatalog | null = null; let resetModelOverride = false; let resetModelOverrideRef: string | undefined; + let resetModelOverrideReason: "disallowed" | "stale" | undefined; const agentEntry = params.agentId ? resolveAgentConfig(cfg, params.agentId) : undefined; + const normalizedDirectStoredOverride = normalizeStoredOverrideModel({ + providerOverride: sessionEntry?.providerOverride, + modelOverride: sessionEntry?.modelOverride, + }); const directStoredOverride = resolvePersistedOverrideModelRef({ defaultProvider, - overrideProvider: sessionEntry?.providerOverride, - overrideModel: sessionEntry?.modelOverride, + overrideProvider: normalizedDirectStoredOverride.providerOverride, + overrideModel: normalizedDirectStoredOverride.modelOverride, }); const directStoredModelOverride = directStoredOverride ? { ...directStoredOverride, source: "session" as const } @@ -310,6 +318,7 @@ export async function createModelSelectionState(params: { resetModelOverride = updated; if (updated) { resetModelOverrideRef = key; + resetModelOverrideReason = staleDirectStoredOverride ? "stale" : "disallowed"; } } } @@ -602,6 +611,7 @@ export async function createModelSelectionState(params: { allowedModelCatalog, resetModelOverride, resetModelOverrideRef, + resetModelOverrideReason, resolveThinkingCatalog, resolveDefaultThinkingLevel, resolveDefaultReasoningLevel, diff --git a/src/auto-reply/reply/stored-model-override.ts b/src/auto-reply/reply/stored-model-override.ts index 6b94c97b7295..ce6b83c3e681 100644 --- a/src/auto-reply/reply/stored-model-override.ts +++ b/src/auto-reply/reply/stored-model-override.ts @@ -4,6 +4,7 @@ import { hasSessionAutoModelFallbackProvenance } from "../../agents/agent-scope. import { modelKey, normalizeModelRef, + normalizeStoredOverrideModel, resolvePersistedOverrideModelRef, } from "../../agents/model-selection.js"; import { resolveSessionParentSessionKey } from "../../channels/plugins/session-conversation.js"; @@ -39,10 +40,14 @@ export function resolveStoredModelOverride(params: { parentSessionKey?: string; defaultProvider: string; }): StoredModelOverride | null { + const directOverride = normalizeStoredOverrideModel({ + providerOverride: params.sessionEntry?.providerOverride, + modelOverride: params.sessionEntry?.modelOverride, + }); const direct = resolvePersistedOverrideModelRef({ defaultProvider: params.defaultProvider, - overrideProvider: params.sessionEntry?.providerOverride, - overrideModel: params.sessionEntry?.modelOverride, + overrideProvider: directOverride.providerOverride, + overrideModel: directOverride.modelOverride, }); if (direct) { return { ...direct, source: "session" }; @@ -55,10 +60,14 @@ export function resolveStoredModelOverride(params: { return null; } const parentEntry = params.sessionStore[parentKey]; + const normalizedParentOverride = normalizeStoredOverrideModel({ + providerOverride: parentEntry?.providerOverride, + modelOverride: parentEntry?.modelOverride, + }); const parentOverride = resolvePersistedOverrideModelRef({ defaultProvider: params.defaultProvider, - overrideProvider: parentEntry?.providerOverride, - overrideModel: parentEntry?.modelOverride, + overrideProvider: normalizedParentOverride.providerOverride, + overrideModel: normalizedParentOverride.modelOverride, }); if (!parentOverride) { return null; @@ -71,12 +80,20 @@ function resolveModelRefKey(params: { overrideProvider?: string; overrideModel?: string; }): string | null { - const ref = resolvePersistedOverrideModelRef(params); + const normalizedOverride = normalizeStoredOverrideModel({ + providerOverride: params.overrideProvider, + modelOverride: params.overrideModel, + }); + const ref = resolvePersistedOverrideModelRef({ + defaultProvider: params.defaultProvider, + overrideProvider: normalizedOverride.providerOverride, + overrideModel: normalizedOverride.modelOverride, + }); if (!ref) { return null; } - const normalized = normalizeModelRef(ref.provider, ref.model); - return modelKey(normalized.provider, normalized.model); + const normalizedRef = normalizeModelRef(ref.provider, ref.model); + return modelKey(normalizedRef.provider, normalizedRef.model); } /** Detects heartbeat auto-fallback overrides that no longer match the primary model. */