From ab312ec22fbc60142c635d62a90f265f08bf6d2b Mon Sep 17 00:00:00 2001 From: Shakker Date: Sun, 2 Aug 2026 21:31:45 +0100 Subject: [PATCH] fix: order slash model cache updates --- .../pages/chat/chat-command-executor.test.ts | 32 +++++++++ ui/src/pages/chat/chat-command-executor.ts | 66 +++++++++++-------- ui/src/pages/chat/chat-commands.ts | 9 +-- ui/src/pages/chat/chat-send.test.ts | 33 +++++++++- ui/src/pages/chat/chat-settings-patches.ts | 5 +- 5 files changed, 106 insertions(+), 39 deletions(-) diff --git a/ui/src/pages/chat/chat-command-executor.test.ts b/ui/src/pages/chat/chat-command-executor.test.ts index 13c508f1c0a0..53cad84f3b3f 100644 --- a/ui/src/pages/chat/chat-command-executor.test.ts +++ b/ui/src/pages/chat/chat-command-executor.test.ts @@ -151,9 +151,11 @@ describe("executeSlashCommand directives", () => { .mockResolvedValue( createResolvedModelPatch(OPENAI_GPT5_MINI_MODEL.id, OPENAI_GPT5_MINI_MODEL.provider), ); + const setModelOverride = vi.fn(); const sessions = { ...createSessionCapability(client), patch, + setModelOverride, } as SessionCapability; const result = await executeSlashCommandImpl(client, "global", "model", "gpt-5-mini", { @@ -176,6 +178,36 @@ describe("executeSlashCommand directives", () => { deferModelOverride: true, }), ); + expect(setModelOverride).toHaveBeenCalledWith("global", "openai/gpt-5-mini"); + }); + + it("does not publish a slash-command model cache value after its owner retires", async () => { + const client = { request: vi.fn() } as unknown as GatewayBrowserClient; + const setModelOverride = vi.fn(); + const sessions = { + ...createSessionCapability(client), + patch: vi + .fn() + .mockResolvedValue( + createResolvedModelPatch(OPENAI_GPT5_MINI_MODEL.id, OPENAI_GPT5_MINI_MODEL.provider), + ), + setModelOverride, + } as SessionCapability; + + const result = await executeSlashCommandImpl(client, "global", "model", "gpt-5-mini", { + sessions, + sessionAccessSnapshot: { + client, + hello: null, + phase: "connected", + }, + agentId: "work", + ownsModelOverride: () => false, + chatModelCatalog: createModelCatalog(OPENAI_GPT5_MINI_MODEL), + }); + + expect(result.failed).not.toBe(true); + expect(setModelOverride).not.toHaveBeenCalled(); }); it("does not patch through a replacement connection after loading session state", async () => { diff --git a/ui/src/pages/chat/chat-command-executor.ts b/ui/src/pages/chat/chat-command-executor.ts index a8db5c392470..76ca1ae5cd91 100644 --- a/ui/src/pages/chat/chat-command-executor.ts +++ b/ui/src/pages/chat/chat-command-executor.ts @@ -76,6 +76,7 @@ type SlashCommandContext = { sessionsResultAgentId?: string | null; defaultAgentId?: string; agentId?: string; + ownsModelOverride?: () => boolean; }; function assertCurrentSlashCommand(context: SlashCommandContext): void { @@ -303,40 +304,47 @@ async function executeModel( try { const requestedModel = args.trim(); - const [patched, resolvedModelCatalog] = await Promise.all([ - patchSession( - context, - sessionKey, - { - model: requestedModel, + const resolvedModelCatalog = modelCatalog + ? Promise.resolve(modelCatalog) + : loadModelCatalog(client, { allowFailure: true }); + let resolvedOverride: ChatModelOverride | null = null; + await patchSession( + context, + sessionKey, + { + model: requestedModel, + }, + { + deferModelOverride: true, + reconcile: async (result) => { + const resolvedModel = result.resolved?.model ?? requestedModel; + let resolvedValue = resolvePreferredServerChatModelValue( + resolvedModel, + result.resolved?.modelProvider, + await resolvedModelCatalog, + ); + const requestedOverride = createChatModelOverride(requestedModel); + const resolvedProvider = result.resolved?.modelProvider?.trim(); + if ( + requestedOverride?.kind === "qualified" && + resolvedProvider && + resolvedValue && + !resolvedValue.toLowerCase().startsWith(`${resolvedProvider.toLowerCase()}/`) && + requestedOverride.value.toLowerCase().endsWith(`/${resolvedModel.trim().toLowerCase()}`) + ) { + resolvedValue = requestedOverride.value; + } + resolvedOverride = createChatModelOverride(resolvedValue); + if (context.ownsModelOverride?.() !== false) { + context.sessions.setModelOverride(sessionKey, resolvedOverride?.value ?? null); + } }, - { deferModelOverride: true }, - ), - modelCatalog - ? Promise.resolve(modelCatalog) - : loadModelCatalog(client, { allowFailure: true }), - ]); - const resolvedModel = patched.resolved?.model ?? requestedModel; - let resolvedValue = resolvePreferredServerChatModelValue( - resolvedModel, - patched.resolved?.modelProvider, - resolvedModelCatalog, + }, ); - const requestedOverride = createChatModelOverride(requestedModel); - const resolvedProvider = patched.resolved?.modelProvider?.trim(); - if ( - requestedOverride?.kind === "qualified" && - resolvedProvider && - resolvedValue && - !resolvedValue.toLowerCase().startsWith(`${resolvedProvider.toLowerCase()}/`) && - requestedOverride.value.toLowerCase().endsWith(`/${resolvedModel.trim().toLowerCase()}`) - ) { - resolvedValue = requestedOverride.value; - } return { content: t("chat.commandResults.model.set", { model: `\`${requestedModel}\`` }), action: "refresh", - sessionPatch: { modelOverride: createChatModelOverride(resolvedValue) }, + sessionPatch: { modelOverride: resolvedOverride }, }; } catch (err) { return { diff --git a/ui/src/pages/chat/chat-commands.ts b/ui/src/pages/chat/chat-commands.ts index 624bb524072f..f2f3c94b2467 100644 --- a/ui/src/pages/chat/chat-commands.ts +++ b/ui/src/pages/chat/chat-commands.ts @@ -423,6 +423,7 @@ export async function dispatchChatSlashCommand( sessionsResultAgentId: host.sessionsResultAgentId, defaultAgentId: resolveUiDefaultAgentId(host), agentId: target.agentId, + ownsModelOverride: () => isChatCommandModelCacheOwnerCurrent(host, target), }); } catch (err) { if (targetIsCurrent()) { @@ -475,14 +476,6 @@ export async function dispatchChatSlashCommand( } if (result.sessionPatch && "modelOverride" in result.sessionPatch) { - // A route switch on the same Gateway still owns the originating session's - // cache. A replacement connection must not consume this late command result. - if (isChatCommandModelCacheOwnerCurrent(host, target)) { - host.sessions.setModelOverride( - target.sessionKey, - result.sessionPatch.modelOverride?.value ?? null, - ); - } if (targetIsCurrent()) { await host.refreshCurrentSessionTools?.(); } diff --git a/ui/src/pages/chat/chat-send.test.ts b/ui/src/pages/chat/chat-send.test.ts index c5b0edc8162b..67c4572e9312 100644 --- a/ui/src/pages/chat/chat-send.test.ts +++ b/ui/src/pages/chat/chat-send.test.ts @@ -8,6 +8,7 @@ import type { GatewaySessionRow, SessionsListResult } from "../../api/types.ts"; import type { UiSettings } from "../../app/settings.ts"; import { SLASH_COMMANDS } from "../../lib/chat/commands.ts"; import { createSessionCapability } from "../../lib/sessions/index.ts"; +import { createResolvedModelPatch } from "../../test-helpers/chat-model.ts"; import { createStorageMock } from "../../test-helpers/storage.ts"; import { waitForFast } from "../../test-helpers/wait-for.ts"; import { @@ -1804,6 +1805,36 @@ describe("handleSendChat", () => { expect(host.chatQueue).toStrictEqual([]); }); + it("keeps a resolved model reconciliation inside the canonical settings queue", async () => { + const reconcile = createDeferred(); + let patchCount = 0; + const host = makeHost({ + requestHandlers: { + "sessions.patch": () => { + patchCount += 1; + return createResolvedModelPatch(patchCount === 1 ? "gpt-5-mini" : "gpt-5", "openai"); + }, + }, + }); + + const first = patchChatSessionSettings( + host, + host.sessionKey, + { model: "openai/gpt-5-mini" }, + { reconcile: async () => await reconcile.promise }, + ); + await waitForFast(() => expect(patchCount).toBe(1)); + const second = patchChatSessionSettings(host, host.sessionKey, { + model: "openai/gpt-5", + }); + await Promise.resolve(); + + expect(patchCount).toBe(1); + reconcile.resolve(); + await Promise.all([first, second]); + expect(patchCount).toBe(2); + }); + it("keeps waiting when a late picker barrier cannot be persisted", async () => { const queuedText = "do not bypass the late picker"; const history = createDeferred(); @@ -4449,7 +4480,7 @@ describe("handleSendChat", () => { expect(host.chatRunId).toBeNull(); expect(host.lastError).toBe("Second session error"); expect(host.chatError).toBe("Second session error"); - expect(host.sessions.state.modelOverrides[item.sessionKey]).toBe("openai/gpt-5-mini"); + expect(host.sessions.state.modelOverrides[item.sessionKey]).toBeUndefined(); expect(refreshCurrentSessionTools).not.toHaveBeenCalled(); expect(refreshCurrentChat).not.toHaveBeenCalled(); }); diff --git a/ui/src/pages/chat/chat-settings-patches.ts b/ui/src/pages/chat/chat-settings-patches.ts index c561cf8a8648..948c17f7cf3b 100644 --- a/ui/src/pages/chat/chat-settings-patches.ts +++ b/ui/src/pages/chat/chat-settings-patches.ts @@ -166,7 +166,10 @@ export async function patchChatCommandSessionSettings( context: ChatCommandSettingsContext, sessionKey: string, patch: SessionPatch, - options: { deferModelOverride?: boolean } = {}, + options: { + deferModelOverride?: boolean; + reconcile?: (result: SessionsPatchResult) => Promise | void; + } = {}, ): Promise>>> { const result = await patchChatSessionSettings( {