mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 21:07:01 -06:00
fix: fence global model picker ownership
This commit is contained in:
@@ -15,10 +15,12 @@ function sessionsResult(sessions: SessionsListResult["sessions"], ts: number): S
|
||||
|
||||
function deferred<T>() {
|
||||
let resolve: (value: T) => void = () => undefined;
|
||||
const promise = new Promise<T>((next) => {
|
||||
let reject: (error: unknown) => void = () => undefined;
|
||||
const promise = new Promise<T>((next, fail) => {
|
||||
resolve = next;
|
||||
reject = fail;
|
||||
});
|
||||
return { promise, resolve };
|
||||
return { promise, reject, resolve };
|
||||
}
|
||||
|
||||
function createSessions(client: GatewayBrowserClient, key: string) {
|
||||
@@ -328,4 +330,44 @@ describe("session list replacement options", () => {
|
||||
expect(sessions.state.modelOverrides[key]).toBe("openai/gpt-old");
|
||||
sessions.dispose();
|
||||
});
|
||||
|
||||
it.each(["resolve", "reject"] as const)(
|
||||
"does not %s an optimistic model patch into a replacement UI owner",
|
||||
async (outcome) => {
|
||||
const pendingPatch = deferred<unknown>();
|
||||
const request = vi.fn(async (method: string) => {
|
||||
if (method === "sessions.patch") {
|
||||
return await pendingPatch.promise;
|
||||
}
|
||||
throw new Error(`Unexpected request: ${method}`);
|
||||
});
|
||||
const key = "global";
|
||||
const sessions = createSessions({ request } as unknown as GatewayBrowserClient, key);
|
||||
let ownsModelOverride = true;
|
||||
sessions.setModelOverride(key, "openai/gpt-old");
|
||||
|
||||
const operation = sessions.patch(
|
||||
key,
|
||||
{ model: "openai/gpt-agent-a" },
|
||||
{
|
||||
deferListRefresh: true,
|
||||
ownsModelOverride: () => ownsModelOverride,
|
||||
},
|
||||
);
|
||||
expect(sessions.state.modelOverrides[key]).toBe("openai/gpt-agent-a");
|
||||
|
||||
ownsModelOverride = false;
|
||||
sessions.setModelOverride(key, "openai/gpt-agent-b");
|
||||
if (outcome === "resolve") {
|
||||
pendingPatch.resolve({ ok: true, path: "", key, entry: {} });
|
||||
await expect(operation).resolves.toMatchObject({ ok: true, key });
|
||||
} else {
|
||||
pendingPatch.reject(new Error("agent A patch failed"));
|
||||
await expect(operation).rejects.toThrow("agent A patch failed");
|
||||
}
|
||||
|
||||
expect(sessions.state.modelOverrides[key]).toBe("openai/gpt-agent-b");
|
||||
sessions.dispose();
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
@@ -27,6 +27,8 @@ export type SessionPatchOptions = {
|
||||
agentId?: string;
|
||||
/** Let a caller with stricter lifecycle ownership publish the resolved model value. */
|
||||
deferModelOverride?: boolean;
|
||||
/** Keep optimistic model state bound to the UI owner that initiated the patch. */
|
||||
ownsModelOverride?: () => boolean;
|
||||
/** Capture the current connection now, but dispatch only after this tail settles. */
|
||||
waitFor?: Promise<unknown>;
|
||||
/**
|
||||
|
||||
@@ -166,8 +166,9 @@ export function createSessionMutations(host: SessionMutationsHost) {
|
||||
let previousModelOverride: string | null | undefined;
|
||||
let modelPatchStarted = false;
|
||||
const modelPatchToken = Symbol();
|
||||
const ownsModelOverride = () => options.ownsModelOverride?.() !== false;
|
||||
const startModelPatch = () => {
|
||||
if (!managesModelOverride || modelPatchStarted) {
|
||||
if (!managesModelOverride || modelPatchStarted || !ownsModelOverride()) {
|
||||
return;
|
||||
}
|
||||
const pendingModelPatch = pendingModelPatches.get(normalizedKey);
|
||||
@@ -187,7 +188,9 @@ export function createSessionMutations(host: SessionMutationsHost) {
|
||||
const restoreModelOverride = () => {
|
||||
if (modelPatchStarted && pendingModelPatches.get(normalizedKey)?.token === modelPatchToken) {
|
||||
pendingModelPatches.delete(normalizedKey);
|
||||
setModelOverride(key, previousModelOverride);
|
||||
if (ownsModelOverride()) {
|
||||
setModelOverride(key, previousModelOverride);
|
||||
}
|
||||
}
|
||||
};
|
||||
try {
|
||||
@@ -216,7 +219,9 @@ export function createSessionMutations(host: SessionMutationsHost) {
|
||||
pendingModelPatches.get(normalizedKey)?.token === modelPatchToken
|
||||
) {
|
||||
pendingModelPatches.delete(normalizedKey);
|
||||
setModelOverride(key, patchParams.model);
|
||||
if (ownsModelOverride()) {
|
||||
setModelOverride(key, patchParams.model);
|
||||
}
|
||||
}
|
||||
return result;
|
||||
} catch (error) {
|
||||
|
||||
@@ -364,7 +364,10 @@ export async function switchChatModel(
|
||||
if (currentOverride === nextModel) {
|
||||
return true;
|
||||
}
|
||||
const previousModelOverride = host.sessions.state.modelOverrides[targetSessionKey];
|
||||
const modelOwnerAgentId = scopedAgentParamsForSession(host, targetSessionKey).agentId;
|
||||
const ownsModelOverride = () =>
|
||||
!isUiGlobalSessionKey(targetSessionKey) ||
|
||||
scopedAgentParamsForSession(host, targetSessionKey).agentId === modelOwnerAgentId;
|
||||
setChatError(host, null, true);
|
||||
const switchPromiseRef: { current?: Promise<boolean> } = {};
|
||||
const clearPendingSwitch = () => {
|
||||
@@ -384,6 +387,7 @@ export async function switchChatModel(
|
||||
},
|
||||
{
|
||||
...scopedAgentParamsForSession(host, targetSessionKey),
|
||||
ownsModelOverride,
|
||||
reconcile: async () => {
|
||||
await host.onModelChanged?.();
|
||||
await refreshCurrentChatSessionList(host);
|
||||
@@ -395,8 +399,9 @@ export async function switchChatModel(
|
||||
}
|
||||
return true;
|
||||
} catch (err) {
|
||||
host.sessions.setModelOverride(targetSessionKey, previousModelOverride);
|
||||
setChatError(host, `Failed to set model: ${String(err)}`, true);
|
||||
if (ownsModelOverride()) {
|
||||
setChatError(host, `Failed to set model: ${String(err)}`, true);
|
||||
}
|
||||
return false;
|
||||
} finally {
|
||||
clearPendingSwitch();
|
||||
|
||||
@@ -116,6 +116,7 @@ export function patchChatSessionSettings(
|
||||
options: {
|
||||
agentId?: string;
|
||||
deferModelOverride?: boolean;
|
||||
ownsModelOverride?: () => boolean;
|
||||
reconcile?: (result: SessionsPatchResult) => Promise<void> | void;
|
||||
} = {},
|
||||
): Promise<SessionsPatchResult | null> {
|
||||
@@ -127,6 +128,7 @@ export function patchChatSessionSettings(
|
||||
const result = await host.sessions.patch(sessionKey, patch, {
|
||||
agentId: options.agentId,
|
||||
deferModelOverride: options.deferModelOverride,
|
||||
ownsModelOverride: options.ownsModelOverride,
|
||||
waitFor: previous,
|
||||
});
|
||||
if (result) {
|
||||
@@ -168,6 +170,7 @@ export async function patchChatCommandSessionSettings(
|
||||
patch: SessionPatch,
|
||||
options: {
|
||||
deferModelOverride?: boolean;
|
||||
ownsModelOverride?: () => boolean;
|
||||
reconcile?: (result: SessionsPatchResult) => Promise<void> | void;
|
||||
} = {},
|
||||
): Promise<NonNullable<Awaited<ReturnType<SessionCapability["patch"]>>>> {
|
||||
|
||||
@@ -6031,6 +6031,66 @@ describe("chat model controls", () => {
|
||||
expect(host.chatThinkingLevel).toBe("high");
|
||||
});
|
||||
|
||||
it("does not restore or report a failed global model switch after the selected agent changes", async () => {
|
||||
const modelPatch = createDeferred<unknown>();
|
||||
const modelOverrides: Record<string, string | null> = {
|
||||
global: "openai/gpt-agent-a-old",
|
||||
};
|
||||
let patchOptions: SessionPatchOptions | undefined;
|
||||
const sessions = {
|
||||
state: { modelOverrides },
|
||||
patch: vi.fn(
|
||||
async (_key: string, _patch: Record<string, unknown>, options?: SessionPatchOptions) => {
|
||||
patchOptions = options;
|
||||
return await modelPatch.promise;
|
||||
},
|
||||
),
|
||||
refresh: async () => {},
|
||||
setModelOverride: vi.fn((key: string, value: string | null | undefined) => {
|
||||
if (value === undefined) {
|
||||
delete modelOverrides[key];
|
||||
} else {
|
||||
modelOverrides[key] = value;
|
||||
}
|
||||
}),
|
||||
patchRowLocal: vi.fn(),
|
||||
};
|
||||
const host = {
|
||||
assistantAgentId: "work",
|
||||
agentsList: { defaultId: "main", scope: "global" },
|
||||
client: {},
|
||||
connected: true,
|
||||
sessionKey: "global",
|
||||
chatModelCatalog: [],
|
||||
chatModelSwitchPromises: {},
|
||||
chatThinkingLevel: null,
|
||||
sessions,
|
||||
sessionsResult: createSessionsResultFromRows([
|
||||
{
|
||||
key: "global",
|
||||
kind: "direct",
|
||||
updatedAt: 1,
|
||||
model: "gpt-agent-a-old",
|
||||
modelProvider: "openai",
|
||||
},
|
||||
]),
|
||||
} as unknown as Parameters<typeof switchChatModel>[0];
|
||||
|
||||
const switching = switchChatModel(host, "openai/gpt-agent-a-new");
|
||||
await waitForFast(() => expect(patchOptions).toBeDefined());
|
||||
expect(patchOptions?.ownsModelOverride?.()).toBe(true);
|
||||
|
||||
host.assistantAgentId = "main";
|
||||
sessions.setModelOverride("global", "openai/gpt-agent-b");
|
||||
modelPatch.reject(new Error("agent A patch failed"));
|
||||
|
||||
await expect(switching).resolves.toBe(false);
|
||||
expect(patchOptions?.ownsModelOverride?.()).toBe(false);
|
||||
expect(modelOverrides.global).toBe("openai/gpt-agent-b");
|
||||
expect(host.lastError ?? null).toBeNull();
|
||||
expect(host.chatError ?? null).toBeNull();
|
||||
});
|
||||
|
||||
it("keeps the newest speed selection when an older patch fails late", async () => {
|
||||
const pendingPatches: Array<{ resolve: () => void; reject: (error: Error) => void }> = [];
|
||||
// Minimal host: the factory's mock gateway rebuilds session rows on every
|
||||
|
||||
Reference in New Issue
Block a user