mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-28 17:43:05 +00:00
fix: use explicit flag for live model switch detection in fallback chain
Replace the ambiguous comparison-based approach (hasDifferentLiveSessionModelSelection
+ in-memory map EMBEDDED_RUN_MODEL_SWITCH_REQUESTS) with a persisted
`liveModelSwitchPending` flag on SessionEntry.
The root cause: the in-memory map was never populated in production because
requestLiveSessionModelSwitch() was removed in commit 622b91d04e and replaced
with refreshQueuedFollowupSession(). This left the comparison-based detection
as the only path, which could not distinguish user-initiated model switches
(via /model command) from system-initiated fallback rotations.
The fix:
- Add `liveModelSwitchPending?: boolean` to SessionEntry (persisted)
- Set the flag to true ONLY when /model command applies a model override
- New `shouldSwitchToLiveModel()` checks the flag + model mismatch together
- New `clearLiveModelSwitchPending()` resets the flag after consumption
- Replace throw-site logic in run.ts to use the new flag-based functions
- Remove orphaned resolveCurrentLiveSelection helper
Only the /model command sets this flag, so system-initiated fallback rotations
are never mistaken for user-initiated model switches. This restores the
live-switch-during-active-run feature that was accidentally broken.
Fixes #57857, #57760, #58137
This commit is contained in:
committed by
Peter Steinberger
parent
678e9e6078
commit
251e086eac
@@ -9,6 +9,7 @@ const state = vi.hoisted(() => ({
|
||||
resolvePersistedModelRefMock: vi.fn(),
|
||||
loadSessionStoreMock: vi.fn(),
|
||||
resolveStorePathMock: vi.fn(),
|
||||
updateSessionStoreMock: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("./pi-embedded.js", () => ({
|
||||
@@ -31,6 +32,7 @@ vi.mock("./model-selection.js", () => ({
|
||||
vi.mock("../config/sessions.js", () => ({
|
||||
loadSessionStore: (...args: unknown[]) => state.loadSessionStoreMock(...args),
|
||||
resolveStorePath: (...args: unknown[]) => state.resolveStorePathMock(...args),
|
||||
updateSessionStore: (...args: unknown[]) => state.updateSessionStoreMock(...args),
|
||||
}));
|
||||
|
||||
async function loadModule() {
|
||||
@@ -94,6 +96,14 @@ describe("live model switch", () => {
|
||||
);
|
||||
state.loadSessionStoreMock.mockReset().mockReturnValue({});
|
||||
state.resolveStorePathMock.mockReset().mockReturnValue("/tmp/session-store.json");
|
||||
state.updateSessionStoreMock
|
||||
.mockReset()
|
||||
.mockImplementation(
|
||||
async (_path: string, updater: (store: Record<string, unknown>) => void) => {
|
||||
const store: Record<string, unknown> = {};
|
||||
updater(store);
|
||||
},
|
||||
);
|
||||
});
|
||||
it("resolves persisted session overrides ahead of agent defaults", async () => {
|
||||
state.loadSessionStoreMock.mockReturnValue({
|
||||
@@ -262,4 +272,147 @@ describe("live model switch", () => {
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
describe("shouldSwitchToLiveModel", () => {
|
||||
it("returns the persisted selection when liveModelSwitchPending is true and model differs", async () => {
|
||||
state.loadSessionStoreMock.mockReturnValue({
|
||||
main: {
|
||||
liveModelSwitchPending: true,
|
||||
providerOverride: "openai",
|
||||
modelOverride: "gpt-5.4",
|
||||
},
|
||||
});
|
||||
|
||||
const { shouldSwitchToLiveModel } = await loadModule();
|
||||
|
||||
const result = shouldSwitchToLiveModel({
|
||||
cfg: { session: { store: "/tmp/custom-store.json" } },
|
||||
sessionKey: "main",
|
||||
agentId: "reply",
|
||||
defaultProvider: "anthropic",
|
||||
defaultModel: "claude-opus-4-6",
|
||||
currentProvider: "anthropic",
|
||||
currentModel: "claude-opus-4-6",
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
provider: "openai",
|
||||
model: "gpt-5.4",
|
||||
authProfileId: undefined,
|
||||
authProfileIdSource: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
it("returns undefined when liveModelSwitchPending is false", async () => {
|
||||
state.loadSessionStoreMock.mockReturnValue({
|
||||
main: {
|
||||
providerOverride: "openai",
|
||||
modelOverride: "gpt-5.4",
|
||||
},
|
||||
});
|
||||
|
||||
const { shouldSwitchToLiveModel } = await loadModule();
|
||||
|
||||
const result = shouldSwitchToLiveModel({
|
||||
cfg: { session: { store: "/tmp/custom-store.json" } },
|
||||
sessionKey: "main",
|
||||
agentId: "reply",
|
||||
defaultProvider: "anthropic",
|
||||
defaultModel: "claude-opus-4-6",
|
||||
currentProvider: "anthropic",
|
||||
currentModel: "claude-opus-4-6",
|
||||
});
|
||||
|
||||
expect(result).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns undefined when liveModelSwitchPending is true but models match", async () => {
|
||||
state.loadSessionStoreMock.mockReturnValue({
|
||||
main: {
|
||||
liveModelSwitchPending: true,
|
||||
providerOverride: "anthropic",
|
||||
modelOverride: "claude-opus-4-6",
|
||||
},
|
||||
});
|
||||
|
||||
const { shouldSwitchToLiveModel } = await loadModule();
|
||||
|
||||
const result = shouldSwitchToLiveModel({
|
||||
cfg: { session: { store: "/tmp/custom-store.json" } },
|
||||
sessionKey: "main",
|
||||
agentId: "reply",
|
||||
defaultProvider: "anthropic",
|
||||
defaultModel: "claude-opus-4-6",
|
||||
currentProvider: "anthropic",
|
||||
currentModel: "claude-opus-4-6",
|
||||
});
|
||||
|
||||
expect(result).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns undefined when sessionKey is missing", async () => {
|
||||
const { shouldSwitchToLiveModel } = await loadModule();
|
||||
|
||||
const result = shouldSwitchToLiveModel({
|
||||
cfg: { session: { store: "/tmp/custom-store.json" } },
|
||||
sessionKey: undefined,
|
||||
agentId: "reply",
|
||||
defaultProvider: "anthropic",
|
||||
defaultModel: "claude-opus-4-6",
|
||||
currentProvider: "anthropic",
|
||||
currentModel: "claude-opus-4-6",
|
||||
});
|
||||
|
||||
expect(result).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("clearLiveModelSwitchPending", () => {
|
||||
it("calls updateSessionStore to clear the flag", async () => {
|
||||
const { clearLiveModelSwitchPending } = await loadModule();
|
||||
|
||||
await clearLiveModelSwitchPending({
|
||||
cfg: { session: { store: "/tmp/custom-store.json" } },
|
||||
sessionKey: "main",
|
||||
agentId: "reply",
|
||||
});
|
||||
|
||||
expect(state.updateSessionStoreMock).toHaveBeenCalledTimes(1);
|
||||
expect(state.resolveStorePathMock).toHaveBeenCalledWith("/tmp/custom-store.json", {
|
||||
agentId: "reply",
|
||||
});
|
||||
});
|
||||
|
||||
it("deletes liveModelSwitchPending from the session entry", async () => {
|
||||
const sessionEntry = { liveModelSwitchPending: true, sessionId: "s-1" };
|
||||
state.updateSessionStoreMock.mockImplementation(
|
||||
async (_path: string, updater: (store: Record<string, unknown>) => void) => {
|
||||
const store: Record<string, typeof sessionEntry> = { main: sessionEntry };
|
||||
updater(store);
|
||||
},
|
||||
);
|
||||
|
||||
const { clearLiveModelSwitchPending } = await loadModule();
|
||||
|
||||
await clearLiveModelSwitchPending({
|
||||
cfg: { session: { store: "/tmp/custom-store.json" } },
|
||||
sessionKey: "main",
|
||||
agentId: "reply",
|
||||
});
|
||||
|
||||
expect(sessionEntry).not.toHaveProperty("liveModelSwitchPending");
|
||||
});
|
||||
|
||||
it("is a no-op when sessionKey is missing", async () => {
|
||||
const { clearLiveModelSwitchPending } = await loadModule();
|
||||
|
||||
await clearLiveModelSwitchPending({
|
||||
cfg: { session: { store: "/tmp/custom-store.json" } },
|
||||
sessionKey: undefined,
|
||||
agentId: "reply",
|
||||
});
|
||||
|
||||
expect(state.updateSessionStoreMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { resolveStorePath } from "../config/sessions/paths.js";
|
||||
import { loadSessionStore } from "../config/sessions/store.js";
|
||||
import { loadSessionStore, updateSessionStore } from "../config/sessions/store.js";
|
||||
import type { SessionEntry } from "../config/sessions/types.js";
|
||||
import { LiveSessionModelSwitchError } from "./live-model-switch-error.js";
|
||||
import { resolveDefaultModelForAgent, resolvePersistedModelRef } from "./model-selection.js";
|
||||
@@ -112,3 +112,87 @@ export function shouldTrackPersistedLiveSessionModelSelection(
|
||||
): boolean {
|
||||
return !hasDifferentLiveSessionModelSelection(current, persisted);
|
||||
}
|
||||
|
||||
/**
|
||||
* Check whether a user-initiated live model switch is pending for the given
|
||||
* session. Returns the persisted model selection when the session's
|
||||
* `liveModelSwitchPending` flag is `true` AND the persisted selection differs
|
||||
* from the currently running model; otherwise returns `undefined`.
|
||||
*
|
||||
* This replaces the previous approach that used an in-memory map
|
||||
* (`consumeEmbeddedRunModelSwitch`) which could not distinguish between
|
||||
* user-initiated `/model` switches and system-initiated fallback rotations.
|
||||
*/
|
||||
export function shouldSwitchToLiveModel(params: {
|
||||
cfg?: { session?: { store?: string } } | undefined;
|
||||
sessionKey?: string;
|
||||
agentId?: string;
|
||||
defaultProvider: string;
|
||||
defaultModel: string;
|
||||
currentProvider: string;
|
||||
currentModel: string;
|
||||
currentAuthProfileId?: string;
|
||||
currentAuthProfileIdSource?: string;
|
||||
}): LiveSessionModelSelection | undefined {
|
||||
const sessionKey = params.sessionKey?.trim();
|
||||
const cfg = params.cfg;
|
||||
if (!cfg || !sessionKey) {
|
||||
return undefined;
|
||||
}
|
||||
const storePath = resolveStorePath(cfg.session?.store, {
|
||||
agentId: params.agentId?.trim(),
|
||||
});
|
||||
const entry = loadSessionStore(storePath, { skipCache: true })[sessionKey];
|
||||
if (!entry?.liveModelSwitchPending) {
|
||||
return undefined;
|
||||
}
|
||||
const persisted = resolveLiveSessionModelSelection({
|
||||
cfg,
|
||||
sessionKey,
|
||||
agentId: params.agentId,
|
||||
defaultProvider: params.defaultProvider,
|
||||
defaultModel: params.defaultModel,
|
||||
});
|
||||
if (
|
||||
!hasDifferentLiveSessionModelSelection(
|
||||
{
|
||||
provider: params.currentProvider,
|
||||
model: params.currentModel,
|
||||
authProfileId: params.currentAuthProfileId,
|
||||
authProfileIdSource: params.currentAuthProfileIdSource,
|
||||
},
|
||||
persisted,
|
||||
)
|
||||
) {
|
||||
return undefined;
|
||||
}
|
||||
return persisted ?? undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Clear the `liveModelSwitchPending` flag from the session entry on disk so
|
||||
* subsequent retry iterations do not re-trigger the switch.
|
||||
*/
|
||||
export async function clearLiveModelSwitchPending(params: {
|
||||
cfg?: { session?: { store?: string } } | undefined;
|
||||
sessionKey?: string;
|
||||
agentId?: string;
|
||||
}): Promise<void> {
|
||||
const sessionKey = params.sessionKey?.trim();
|
||||
const cfg = params.cfg;
|
||||
if (!cfg || !sessionKey) {
|
||||
return;
|
||||
}
|
||||
const storePath = resolveStorePath(cfg.session?.store, {
|
||||
agentId: params.agentId?.trim(),
|
||||
});
|
||||
if (!storePath) {
|
||||
return;
|
||||
}
|
||||
await updateSessionStore(storePath, (store) => {
|
||||
const entry = store[sessionKey];
|
||||
if (entry) {
|
||||
delete entry.liveModelSwitchPending;
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
@@ -27,8 +27,8 @@ import {
|
||||
} from "../failover-error.js";
|
||||
import { LiveSessionModelSwitchError } from "../live-model-switch-error.js";
|
||||
import {
|
||||
hasDifferentLiveSessionModelSelection,
|
||||
consumeLiveSessionModelSwitch,
|
||||
shouldSwitchToLiveModel,
|
||||
clearLiveModelSwitchPending,
|
||||
} from "../live-model-switch.js";
|
||||
import {
|
||||
applyAuthHeaderOverride,
|
||||
@@ -235,12 +235,6 @@ export async function runEmbeddedPiAgent(
|
||||
let lastProfileId: string | undefined;
|
||||
let runtimeAuthState: RuntimeAuthState | null = null;
|
||||
let runtimeAuthRefreshCancelled = false;
|
||||
const resolveCurrentLiveSelection = () => ({
|
||||
provider,
|
||||
model: modelId,
|
||||
authProfileId: preferredProfileId,
|
||||
authProfileIdSource: params.authProfileIdSource,
|
||||
});
|
||||
const {
|
||||
advanceAuthProfile,
|
||||
initializeAuthProfile,
|
||||
@@ -620,12 +614,23 @@ export async function runEmbeddedPiAgent(
|
||||
!attempt.lastToolError &&
|
||||
attempt.toolMetas.length === 0 &&
|
||||
attempt.assistantTexts.length === 0;
|
||||
const requestedSelection = consumeLiveSessionModelSwitch(params.sessionId);
|
||||
if (
|
||||
requestedSelection &&
|
||||
canRestartForLiveSwitch &&
|
||||
hasDifferentLiveSessionModelSelection(resolveCurrentLiveSelection(), requestedSelection)
|
||||
) {
|
||||
const requestedSelection = shouldSwitchToLiveModel({
|
||||
cfg: params.config,
|
||||
sessionKey: params.sessionKey,
|
||||
agentId: params.agentId,
|
||||
defaultProvider: DEFAULT_PROVIDER,
|
||||
defaultModel: DEFAULT_MODEL,
|
||||
currentProvider: provider,
|
||||
currentModel: modelId,
|
||||
currentAuthProfileId: preferredProfileId,
|
||||
currentAuthProfileIdSource: params.authProfileIdSource,
|
||||
});
|
||||
if (requestedSelection && canRestartForLiveSwitch) {
|
||||
await clearLiveModelSwitchPending({
|
||||
cfg: params.config,
|
||||
sessionKey: params.sessionKey,
|
||||
agentId: params.agentId,
|
||||
});
|
||||
log.info(
|
||||
`live session model switch requested during active attempt for ${params.sessionId}: ${provider}/${modelId} -> ${requestedSelection.provider}/${requestedSelection.model}`,
|
||||
);
|
||||
|
||||
@@ -415,6 +415,12 @@ export async function handleDirectiveOnly(
|
||||
profileOverride,
|
||||
});
|
||||
modelSelectionUpdated = applied.updated;
|
||||
if (applied.updated) {
|
||||
// Signal the embedded runner that this is a user-initiated model
|
||||
// switch, so it can distinguish it from system-initiated fallback
|
||||
// rotations and correctly throw LiveSessionModelSwitchError.
|
||||
sessionEntry.liveModelSwitchPending = true;
|
||||
}
|
||||
}
|
||||
if (directives.hasQueueDirective && directives.queueReset) {
|
||||
delete sessionEntry.queueMode;
|
||||
|
||||
@@ -132,6 +132,14 @@ export type SessionEntry = {
|
||||
authProfileOverride?: string;
|
||||
authProfileOverrideSource?: "auto" | "user";
|
||||
authProfileOverrideCompactionCount?: number;
|
||||
/**
|
||||
* Set to `true` by the `/model` command when the user explicitly switches
|
||||
* models during an active run. The embedded runner checks this flag to
|
||||
* decide whether to throw `LiveSessionModelSwitchError`. System-initiated
|
||||
* fallbacks (rate-limit retry rotation) never set this flag, so they are
|
||||
* never mistaken for user-initiated switches.
|
||||
*/
|
||||
liveModelSwitchPending?: boolean;
|
||||
groupActivation?: "mention" | "always";
|
||||
groupActivationNeedsSystemIntro?: boolean;
|
||||
sendPolicy?: "allow" | "deny";
|
||||
|
||||
Reference in New Issue
Block a user