diff --git a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift index fb7400ebcc74..20eefcef556b 100644 --- a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift +++ b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift @@ -6312,6 +6312,7 @@ public struct SessionRow: Codable, Sendable { public let estimatedcostusd: Double? public let model: String? public let modelprovider: String? + public let modeloverridesource: AnyCodable? public let tooloverrides: [String: AnyCodable]? public init( @@ -6384,6 +6385,7 @@ public struct SessionRow: Codable, Sendable { estimatedcostusd: Double? = nil, model: String? = nil, modelprovider: String? = nil, + modeloverridesource: AnyCodable? = nil, tooloverrides: [String: AnyCodable]? = nil) { self.key = key @@ -6455,6 +6457,7 @@ public struct SessionRow: Codable, Sendable { self.estimatedcostusd = estimatedcostusd self.model = model self.modelprovider = modelprovider + self.modeloverridesource = modeloverridesource self.tooloverrides = tooloverrides } @@ -6528,6 +6531,7 @@ public struct SessionRow: Codable, Sendable { case estimatedcostusd = "estimatedCostUsd" case model case modelprovider = "modelProvider" + case modeloverridesource = "modelOverrideSource" case tooloverrides = "toolOverrides" } } diff --git a/packages/gateway-protocol/src/schema/sessions-row.test.ts b/packages/gateway-protocol/src/schema/sessions-row.test.ts index 93fcd981dfa2..faa634163b25 100644 --- a/packages/gateway-protocol/src/schema/sessions-row.test.ts +++ b/packages/gateway-protocol/src/schema/sessions-row.test.ts @@ -83,4 +83,24 @@ describe("SessionRowSchema", () => { expect(accepted.every(validateSessionsAssignOwnerParams)).toBe(true); expect(rejected.every((value) => !validateSessionsAssignOwnerParams(value))).toBe(true); }); + + it.each(["user", "auto", null] as const)("accepts model override source %s", (source) => { + expect( + Value.Check(SessionRowSchema, { + key: "agent:main:main", + kind: "global", + modelOverrideSource: source, + }), + ).toBe(true); + }); + + it("rejects an invalid model override source", () => { + expect( + Value.Check(SessionRowSchema, { + key: "agent:main:main", + kind: "global", + modelOverrideSource: "session", + }), + ).toBe(false); + }); }); diff --git a/packages/gateway-protocol/src/schema/sessions-row.ts b/packages/gateway-protocol/src/schema/sessions-row.ts index ced001ba3e37..0c28b12f70ea 100644 --- a/packages/gateway-protocol/src/schema/sessions-row.ts +++ b/packages/gateway-protocol/src/schema/sessions-row.ts @@ -152,6 +152,10 @@ export const SessionRowSchema = Type.Object( estimatedCostUsd: Type.Optional(Type.Number()), model: Type.Optional(Type.String()), modelProvider: Type.Optional(Type.String()), + /** Persisted override provenance; null means inherited, omission means not projected. */ + modelOverrideSource: Type.Optional( + Type.Union([Type.Literal("user"), Type.Literal("auto"), Type.Null()]), + ), toolOverrides: Type.Optional(SessionToolOverridesSchema), }, { additionalProperties: true }, diff --git a/src/agents/openclaw-tools.session-status.test.ts b/src/agents/openclaw-tools.session-status.test.ts index db8beed94cbd..1ca0ed73a7da 100644 --- a/src/agents/openclaw-tools.session-status.test.ts +++ b/src/agents/openclaw-tools.session-status.test.ts @@ -618,40 +618,51 @@ describe("session_status tool", () => { expect(details.statusText).not.toContain("OAuth/token status"); expect(tool.outputSchema).toBeDefined(); expect(Value.Check(tool.outputSchema!, result.details)).toBe(true); - expect(compactToolOutputHint(tool.outputSchema)).toBe( - '{ changedModel: boolean; ok: true; sessionKey: string; stateVersion: number; statusText: string; active?: { accountId?: string; channel?: string; threadId?: string | number; to?: string }; deliveryContext?: { accountId?: string; channel?: string; threadId?: string | number; to?: string }; model?: string; modelOverride?: string | null; modelProvider?: string; origin?: { accountId?: string; provider?: string; threadId?: string | number }; stateChanges?: { earliestAvailableSequence: number; events: Array<{ actorType: "human" | "agent" | "system"; kind: string; occurredAt: number; sequence: number; summary: string; actorId?: string; payload?: { channel?: string; outcome?: "error" | "timeout" | "cancelled"; turns?: number }; runId?: string }>; historyGap: boolean; truncated: boolean } }', - ); + // The full contract exceeds the compact hint budget; never promote a truncated shape. + expect(compactToolOutputHint(tool.outputSchema)).toBeUndefined(); }); - it("uses the persisted fixed-store owner for a bare current session", async () => { - resetSessionStore({ - global: { - sessionId: "ops-global", - updatedAt: 10, - }, - }); - mockConfig = { - session: { mainKey: "main", scope: "global", store: "/tmp/shared-sessions.sqlite" }, - agents: { - ownership: "explicit", - defaults: { - model: { primary: "openai/gpt-5.4" }, - models: {}, - sessionStore: { agentId: "ops" }, + it.each([false, true])( + "reports the fixed-store owner for a bare current session (reset: %s)", + async (reset) => { + resetSessionStore({ + global: { + sessionId: "ops-global", + updatedAt: 10, + providerOverride: "anthropic", + modelOverride: "claude-sonnet-4-6", }, - entries: { ops: {}, research: {} }, - }, - tools: { agentToAgent: { enabled: false } }, - }; + }); + mockConfig = { + session: { mainKey: "main", scope: "global", store: "/tmp/shared-sessions.sqlite" }, + agents: { + ownership: "explicit", + defaults: { + model: { primary: "openai/gpt-5.4" }, + models: {}, + sessionStore: { agentId: "ops" }, + }, + entries: { ops: {}, research: {} }, + }, + tools: { agentToAgent: { enabled: false } }, + }; - const result = await createSessionStatusTool({ - agentSessionKey: "global", - config: mockConfig as never, - }).execute("owned-global", {}); + const tool = createSessionStatusTool({ + agentSessionKey: "global", + config: mockConfig as never, + }); + const result = await tool.execute("owned-global", reset ? { model: "default" } : {}); - expect(result.details).toMatchObject({ ok: true, sessionKey: "global" }); - expect(getSessionStateVersionMock).toHaveBeenCalledWith("global", "ops"); - }); + expect(result.details).toMatchObject({ + ok: true, + sessionKey: "global", + agentId: "ops", + changedModel: reset, + }); + expect(Value.Check(tool.outputSchema!, result.details)).toBe(true); + expect(getSessionStateVersionMock).toHaveBeenCalledWith("global", "ops"); + }, + ); it("does not treat another agent's fixed-store bare key as self", async () => { resetSessionStore({ diff --git a/src/agents/tools/session-status-tool.ts b/src/agents/tools/session-status-tool.ts index 8bb93072534a..0b9003354d10 100644 --- a/src/agents/tools/session-status-tool.ts +++ b/src/agents/tools/session-status-tool.ts @@ -157,6 +157,7 @@ const SessionStatusOutputSchema = Type.Object( { ok: Type.Literal(true), sessionKey: Type.String(), + agentId: Type.String(), changedModel: Type.Boolean(), stateVersion: Type.Integer(), statusText: Type.String(), @@ -1222,6 +1223,7 @@ export function createSessionStatusTool(opts?: { details: { ok: true, sessionKey: scopedResolved.key, + agentId, changedModel, stateVersion, ...(stateChanges ? { stateChanges } : {}), diff --git a/src/config/sessions/model-override-provenance.test.ts b/src/config/sessions/model-override-provenance.test.ts index 3c5e53509164..19d8607ad2a0 100644 --- a/src/config/sessions/model-override-provenance.test.ts +++ b/src/config/sessions/model-override-provenance.test.ts @@ -1,5 +1,41 @@ import { describe, expect, it } from "vitest"; -import { hasSessionActiveAutoModelFallback } from "./model-override-provenance.js"; +import { + hasSessionActiveAutoModelFallback, + resolveSessionModelOverrideSource, +} from "./model-override-provenance.js"; + +describe("resolveSessionModelOverrideSource", () => { + it.each([ + { name: "inherited selection", entry: undefined, expected: null }, + { + name: "explicit user pin", + entry: { modelOverride: "gpt-5.6-sol", modelOverrideSource: "user" as const }, + expected: "user", + }, + { + name: "automatic fallback", + entry: { modelOverride: "fallback", modelOverrideSource: "auto" as const }, + expected: "auto", + }, + { + name: "legacy user pin", + entry: { providerOverride: "openai", modelOverride: "gpt-5.6-sol" }, + expected: "user", + }, + { + name: "legacy automatic fallback", + entry: { + providerOverride: "fallback", + modelOverride: "secondary", + modelOverrideFallbackOriginProvider: "primary", + modelOverrideFallbackOriginModel: "main", + }, + expected: "auto", + }, + ])("returns $expected for $name", ({ entry, expected }) => { + expect(resolveSessionModelOverrideSource(entry)).toBe(expected); + }); +}); describe("hasSessionActiveAutoModelFallback", () => { it.each([ diff --git a/src/config/sessions/model-override-provenance.ts b/src/config/sessions/model-override-provenance.ts index 4330b767ce2a..fa7a949a0b03 100644 --- a/src/config/sessions/model-override-provenance.ts +++ b/src/config/sessions/model-override-provenance.ts @@ -25,6 +25,28 @@ export function hasSessionAutoModelFallbackProvenance( ); } +/** Resolves override source while normalizing entries written before source tracking. */ +export function resolveSessionModelOverrideSource( + entry: + | Pick< + SessionEntry, + | "providerOverride" + | "modelOverride" + | "modelOverrideSource" + | "modelOverrideFallbackOriginProvider" + | "modelOverrideFallbackOriginModel" + > + | undefined, +): "auto" | "user" | null { + if (!normalizeOptionalString(entry?.modelOverride)) { + return null; + } + if (entry?.modelOverrideSource) { + return entry.modelOverrideSource; + } + return hasSessionAutoModelFallbackProvenance(entry) ? "auto" : "user"; +} + /** Resolves persisted route provenance, including fallback pins from before the marker existed. */ export function resolveSessionModelOverrideRouteResolution( entry: diff --git a/src/config/sessions/reset-preserved-selection.ts b/src/config/sessions/reset-preserved-selection.ts index bda880f60fc2..f489d17b63d2 100644 --- a/src/config/sessions/reset-preserved-selection.ts +++ b/src/config/sessions/reset-preserved-selection.ts @@ -1,6 +1,6 @@ // Reset preservation keeps user-selected model/auth overrides while dropping automatic fallbacks. import { resolveSessionAuthProfileOverrideSource } from "./auth-profile-override-provenance.js"; -import { hasSessionAutoModelFallbackProvenance } from "./model-override-provenance.js"; +import { resolveSessionModelOverrideSource } from "./model-override-provenance.js"; import type { SessionEntry } from "./types.js"; type ResetPreservedSelectionState = Pick< @@ -34,16 +34,7 @@ export function resolveResetPreservedSelection(params: { } const preserved: Partial = {}; - const recoveredAutoFallbackOverride = - entry.modelOverrideSource === undefined && hasSessionAutoModelFallbackProvenance(entry); - // Missing source on older entries means "user" unless fallback provenance proves the runtime - // created the override automatically. - const preserveLegacyUserModelOverride = - entry.modelOverrideSource === "user" || - (entry.modelOverrideSource === undefined && - Boolean(entry.modelOverride) && - !recoveredAutoFallbackOverride); - if (preserveLegacyUserModelOverride && entry.modelOverride) { + if (resolveSessionModelOverrideSource(entry) === "user" && entry.modelOverride) { preserved.providerOverride = entry.providerOverride; preserved.modelOverride = entry.modelOverride; preserved.modelOverrideSource = "user"; diff --git a/src/gateway/session-event-payload.test.ts b/src/gateway/session-event-payload.test.ts index 907153685056..1cc2a16c1220 100644 --- a/src/gateway/session-event-payload.test.ts +++ b/src/gateway/session-event-payload.test.ts @@ -162,3 +162,39 @@ it("preserves active run id ownership across omitted, liveness, and exact states session: { hasActiveRun: true, activeRunIds: ["run-1"] }, }); }); + +it.each(["user", "auto", null] as const)( + "carries model override source %s into session change events", + (source) => { + expect( + buildGatewaySessionEventFields({ + sessionRow: { + key: "agent:main:pinned", + kind: "direct", + updatedAt: 1, + modelOverrideSource: source, + }, + }).modelOverrideSource, + ).toBe(source); + }, +); + +it.each(["user", "auto", null] as const)( + "does not mix lifecycle snapshots with model source %s", + (modelOverrideSource) => { + const snapshot = buildGatewaySessionSnapshot({ + sessionRow: { + key: "agent:main:pinned", + kind: "direct", + updatedAt: 1, + model: "model-a", + modelProvider: "provider", + modelOverrideSource, + }, + lifecycle: true, + includeSession: true, + }); + expect(snapshot.modelOverrideSource).toBeUndefined(); + expect(snapshot.session).not.toHaveProperty("modelOverrideSource"); + }, +); diff --git a/src/gateway/session-event-payload.ts b/src/gateway/session-event-payload.ts index ae18d2524620..868a34455ade 100644 --- a/src/gateway/session-event-payload.ts +++ b/src/gateway/session-event-payload.ts @@ -105,6 +105,7 @@ export function buildGatewaySessionEventFields(params: { effectiveResponseUsage: sessionRow.effectiveResponseUsage, modelProvider: sessionRow.modelProvider, model: sessionRow.model, + modelOverrideSource: sessionRow.modelOverrideSource, agentRuntime: sessionRow.agentRuntime, status: params.status ?? sessionRow.status, // Explicit null lets subscribed clients clear the previous run's failure reason. @@ -161,6 +162,7 @@ export function buildGatewaySessionSnapshot(params: { if (params.lifecycle) { delete sessionRow.modelProvider; delete sessionRow.model; + delete sessionRow.modelOverrideSource; delete sessionRow.agentRuntime; if (sessionRow.totalTokensFresh !== true) { delete sessionRow.totalTokens; diff --git a/src/gateway/session-utils-owners.test.ts b/src/gateway/session-utils-owners.test.ts index 1303b8202722..1bca4cca484d 100644 --- a/src/gateway/session-utils-owners.test.ts +++ b/src/gateway/session-utils-owners.test.ts @@ -570,7 +570,7 @@ it("keeps the serialized list response deterministic for the current filter path const expectedSerializedResponse = [ '{"ts":1000000,"path":"/tmp/openclaw-session-byte-parity","count":1,"totalCount":1,"limitApplied":100,"nextOffset":null,"hasMore":false,"owners":[]', ',"defaults":{"modelProvider":"openai","model":"gpt-5.4","contextTokens":200000,"agentRuntime":{"id":"codex","cloudPlacementSupported":false,"devicePlacementSupported":false,"source":"implicit"},"thinkingLevels":[{"id":"off","label":"off"},{"id":"minimal","label":"minimal"},{"id":"low","label":"low"},{"id":"medium","label":"medium"},{"id":"high","label":"high"},{"id":"xhigh","label":"xhigh"}],"thinkingOptions":["off","minimal","low","medium","high","xhigh"],"thinkingDefault":"off"}', - ',"sessions":[{"key":"global","visibility":"shared","createdActor":{"type":"system","id":"creator-b"},"kind":"global","classification":"global","agentId":"main","isMain":false,"isBackground":false,"subject":"needle global","updatedAt":999999,"archived":false,"pinned":false,"unread":false,"sessionId":"session-global","thinkingLevels":[{"id":"off","label":"off"},{"id":"minimal","label":"minimal"},{"id":"low","label":"low"},{"id":"medium","label":"medium"},{"id":"high","label":"high"},{"id":"xhigh","label":"xhigh"}],"thinkingOptions":["off","minimal","low","medium","high","xhigh"],"thinkingDefault":"off","effectiveFastMode":false,"effectiveFastModeSource":"default","fastAutoOnSeconds":60,"totalTokens":1,"totalTokensFresh":true,"estimatedCostUsd":0,"effectiveResponseUsage":"off","effectiveQueueMode":"steer","modelProvider":"openai","model":"gpt-5.4","agentRuntime":{"id":"codex","cloudPlacementSupported":false,"devicePlacementSupported":false,"source":"implicit"},"contextTokens":100}]}', + ',"sessions":[{"key":"global","visibility":"shared","createdActor":{"type":"system","id":"creator-b"},"kind":"global","classification":"global","agentId":"main","isMain":false,"isBackground":false,"subject":"needle global","updatedAt":999999,"archived":false,"pinned":false,"unread":false,"sessionId":"session-global","thinkingLevels":[{"id":"off","label":"off"},{"id":"minimal","label":"minimal"},{"id":"low","label":"low"},{"id":"medium","label":"medium"},{"id":"high","label":"high"},{"id":"xhigh","label":"xhigh"}],"thinkingOptions":["off","minimal","low","medium","high","xhigh"],"thinkingDefault":"off","effectiveFastMode":false,"effectiveFastModeSource":"default","fastAutoOnSeconds":60,"totalTokens":1,"totalTokensFresh":true,"estimatedCostUsd":0,"effectiveResponseUsage":"off","effectiveQueueMode":"steer","modelProvider":"openai","model":"gpt-5.4","modelOverrideSource":null,"agentRuntime":{"id":"codex","cloudPlacementSupported":false,"devicePlacementSupported":false,"source":"implicit"},"contextTokens":100}]}', ].join(""); expect(JSON.stringify(result)).toBe(expectedSerializedResponse); diff --git a/src/gateway/session-utils-row.ts b/src/gateway/session-utils-row.ts index d2184ea3ddf7..6dc3d6d99872 100644 --- a/src/gateway/session-utils-row.ts +++ b/src/gateway/session-utils-row.ts @@ -34,6 +34,7 @@ import { type InternalSessionEntry, type SessionEntry, } from "../config/sessions.js"; +import { resolveSessionModelOverrideSource } from "../config/sessions/model-override-provenance.js"; import { sessionEntryForkedFromParent } from "../config/sessions/session-entry-lineage.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { projectPluginSessionExtensionsSync } from "../plugins/host-hook-state.js"; @@ -668,6 +669,7 @@ export function buildGatewaySessionRow(params: { }).mode, modelProvider: rowModelProvider, model: rowModel, + modelOverrideSource: resolveSessionModelOverrideSource(entry), modelSelectionLocked: entry?.modelSelectionLocked, agentRuntime: projectWorkerPlacementAgentRuntime(thinkingProjection.agentRuntime), contextTokens, diff --git a/src/gateway/session-utils.test.ts b/src/gateway/session-utils.test.ts index 1c0b2a2996bc..856849be3e63 100644 --- a/src/gateway/session-utils.test.ts +++ b/src/gateway/session-utils.test.ts @@ -318,6 +318,50 @@ describe("gateway session utils", () => { afterAll(closeSessionSqliteDatabasesForTest); + test.each([ + { + name: "inherited default", + entry: { sessionId: "inherited-default", updatedAt: 1 }, + expected: null, + }, + { + name: "user pin equal to default", + entry: { + sessionId: "user-pin", + updatedAt: 1, + providerOverride: "openai", + modelOverride: "gpt-5.4", + modelOverrideSource: "user", + }, + expected: "user", + }, + { + name: "automatic fallback", + entry: { + sessionId: "automatic-fallback", + updatedAt: 1, + providerOverride: "openai", + modelOverride: "gpt-5.4-mini", + modelOverrideSource: "auto", + }, + expected: "auto", + }, + ] satisfies Array<{ + name: string; + entry: SessionEntry; + expected: "auto" | "user" | null; + }>)("projects model override source for $name", ({ entry, expected }) => { + const row = buildGatewaySessionRow({ + cfg: createModelDefaultsConfig({ primary: "openai/gpt-5.4" }), + storePath: "", + store: {}, + key: "main", + entry, + }); + + expect(row.modelOverrideSource).toBe(expected); + }); + test.each([ { name: "never read", entry: {}, expected: false }, { diff --git a/src/gateway/session-utils.types.ts b/src/gateway/session-utils.types.ts index bdae190e69f6..bef71db544d0 100644 --- a/src/gateway/session-utils.types.ts +++ b/src/gateway/session-utils.types.ts @@ -190,6 +190,7 @@ export type GatewaySessionRow = { effectiveQueueMode?: QueueMode; modelProvider?: string; model?: string; + modelOverrideSource?: "user" | "auto" | null; modelSelectionLocked?: boolean; agentRuntime?: GatewayAgentRuntime; contextTokens?: number; diff --git a/ui/src/e2e/chat-flow.model-picker-refresh.e2e.test.ts b/ui/src/e2e/chat-flow.model-picker-refresh.e2e.test.ts index c5e85f379f08..50f5feb262b0 100644 --- a/ui/src/e2e/chat-flow.model-picker-refresh.e2e.test.ts +++ b/ui/src/e2e/chat-flow.model-picker-refresh.e2e.test.ts @@ -22,6 +22,63 @@ async function screenshot(page: Page, name: string) { } suite.define(() => { + it("clears a persisted pin matching the default through the default model row", async () => { + const context = await suite.newBrowserContext({ + locale: "en-US", + serviceWorkers: "block", + viewport: { height: 900, width: 1280 }, + ...(captureUiProof + ? { recordVideo: { dir: proofDir, size: { height: 900, width: 1280 } } } + : {}), + }); + const page = await context.newPage(); + const session = { + key: "main", + kind: "direct", + updatedAt: 1, + sessionId: "model-pin-proof", + model: "gpt-5.5", + modelProvider: "openai", + modelOverrideSource: "user", + }; + const gateway = await installMockGateway(page, { + sessionKey: "main", + sessionInfo: session, + models: [{ id: "gpt-5.5", name: "Proof Model", provider: "openai" }], + methodResponses: { + "sessions.list": { + ts: 1, + path: "", + count: 1, + sessions: [session], + defaults: { model: "gpt-5.5", modelProvider: "openai", contextTokens: null }, + }, + }, + }); + try { + await page.goto(`${suite.server.baseUrl}chat`); + const picker = page.locator( + 'openclaw-chat-pane[aria-hidden="false"] .chat-controls__model-picker', + ); + await picker.locator('[data-chat-model-select="true"]').click(); + await picker.getByText("Only for this session", { exact: true }).waitFor(); + await screenshot(page, "03-pin-matching-default.png"); + await picker.getByRole("option", { name: "Proof Model", exact: true }).click(); + const request = await gateway.waitForRequest("sessions.patch"); + expect(request.params).toMatchObject({ key: "main", model: null }); + await expect + .poll(() => + picker.locator('[data-chat-model-select="true"]').getAttribute("data-chat-select-value"), + ) + .toBe(""); + await picker.locator('[data-chat-model-select="true"]').click(); + await expect.poll(() => picker.locator("[data-chat-model-reset]").count()).toBe(0); + await screenshot(page, "04-pin-cleared.png"); + } finally { + await context.close(); + } + }); + it("keeps the warm model list interactive while a picker-open refresh is in flight", async () => { const context = await suite.newBrowserContext({ locale: "en-US", diff --git a/ui/src/i18n/locales/en.ts b/ui/src/i18n/locales/en.ts index 42b24eb841f8..ecd6526c09b4 100644 --- a/ui/src/i18n/locales/en.ts +++ b/ui/src/i18n/locales/en.ts @@ -5809,9 +5809,8 @@ export const en: TranslationMap = { fastMode: "Fast mode", searchModels: "Search models", noMatchingModels: "No models match your search", - sessionOverride: "Session override", - resetToDefault: "Reset to default ({model})", - useDefault: "Use default", + onlyForSession: "Only for this session", + useDefaultModel: "Use default ({model})", defaultWithModel: "Default ({model})", defaultWithLevel: "Default ({level})", fastHelp: "Faster responses, higher usage of limits.", diff --git a/ui/src/lib/chat/model-select-state.test.ts b/ui/src/lib/chat/model-select-state.test.ts index be5b6d20dbca..9df55648e1e2 100644 --- a/ui/src/lib/chat/model-select-state.test.ts +++ b/ui/src/lib/chat/model-select-state.test.ts @@ -684,6 +684,81 @@ describe("chat-model-select-state", () => { expect(resolved.options).toEqual([{ value: "openai/gpt-5.6-sol", label: "GPT-5.6 Sol" }]); }); + // The session model equals the agent default in every case, so anything other than + // the recorded marker would have to guess — and would always guess "inherited". + it.each([ + { name: "inherited default", source: null, expected: null }, + { name: "user pin the default grew into", source: "user" as const, expected: "user" }, + { name: "automatic fallback", source: "auto" as const, expected: "auto" }, + ])("resolves $expected from provenance for $name", ({ source, expected }) => { + const state = createChatModelState({ + agentDefaultModel: "openai/gpt-5.6-sol", + chatModelCatalog: createModelCatalog({ + id: "gpt-5.6-sol", + name: "GPT-5.6 Sol", + provider: "openai", + }), + sessionsResult: createSessionsListResult({ + model: "gpt-5.6-sol", + modelProvider: "openai", + modelOverrideSource: source, + defaultsModel: "gpt-5.6-sol", + defaultsProvider: "openai", + }), + }); + + const resolved = resolveChatModelSelectState(state); + expect(resolved.currentOverride).toBe("openai/gpt-5.6-sol"); + expect(resolved.modelOverrideSource).toBe(expected); + }); + + it("reads pin provenance from a canonical main row when the route uses the alias key", () => { + const state = createChatModelState({ + sessionsResult: createSessionsListResult({ + model: "gpt-5-mini", + modelProvider: "openai", + modelOverrideSource: "user", + }), + }); + // Route key stays the `main` alias while the Gateway reports the canonical row. + expectDefined(state.sessionsResult?.sessions[0], "main session row").key = "agent:main:main"; + + const resolved = resolveChatModelSelectState(state); + + expect(resolved.currentOverride).toBe("openai/gpt-5-mini"); + expect(resolved.modelOverrideSource).toBe("user"); + }); + + // `currentOverride` already lets a pending local selection outrank the row, so + // provenance has to follow it — otherwise the picker would report a model and an + // origin belonging to two different points in time. + it("keeps provenance and the effective model on the same in-flight selection", () => { + const pendingPin = createChatModelState({ + modelOverrides: { main: "openai/gpt-5-mini" }, + sessionsResult: createSessionsListResult({ + model: "gpt-5", + modelProvider: "openai", + modelOverrideSource: null, + }), + }); + + expect(resolveChatModelSelectState(pendingPin).currentOverride).toBe("openai/gpt-5-mini"); + expect(resolveChatModelSelectState(pendingPin).modelOverrideSource).toBe("user"); + + const pendingReset = { + ...pendingPin, + modelOverrides: { main: null }, + sessionsResult: createSessionsListResult({ + model: "gpt-5-mini", + modelProvider: "openai", + modelOverrideSource: "user" as const, + }), + }; + + expect(resolveChatModelSelectState(pendingReset).currentOverride).toBe(""); + expect(resolveChatModelSelectState(pendingReset).modelOverrideSource).toBeNull(); + }); + it("disambiguates duplicate friendly names in picker options and default labels", () => { const state = createChatModelState({ chatModelCatalog: createModelCatalog( diff --git a/ui/src/lib/chat/model-select-state.ts b/ui/src/lib/chat/model-select-state.ts index 78f3027baf24..dbe8fd923b49 100644 --- a/ui/src/lib/chat/model-select-state.ts +++ b/ui/src/lib/chat/model-select-state.ts @@ -36,6 +36,7 @@ type ChatModelSelectState = { currentOverride: string; defaultModel: string; defaultLabel: string; + modelOverrideSource: GatewaySessionRow["modelOverrideSource"]; options: ChatModelSelectOption[]; }; @@ -78,6 +79,15 @@ function resolveActiveSessionRow(state: ChatModelSelectStateInput) { ); } +function resolveModelOverrideSource(state: ChatModelSelectStateInput) { + // A local selection is newer than the row that still reports the previous + // provenance, so it owns the answer until the refreshed row lands. + if (Object.hasOwn(state.modelOverrides, state.sessionKey)) { + return state.modelOverrides[state.sessionKey] == null ? null : "user"; + } + return resolveActiveSessionRow(state)?.modelOverrideSource; +} + export function resolveChatModelOverrideValue(state: ChatModelSelectStateInput): string { const catalog = state.chatModelCatalog ?? []; @@ -216,6 +226,7 @@ export function resolveChatModelSelectState( currentOverride, defaultModel, defaultLabel: defaultModel ? `Default (${defaultLabel})` : "Default model", + modelOverrideSource: resolveModelOverrideSource(state), options, }; } diff --git a/ui/src/lib/sessions/index-subscriptions.test.ts b/ui/src/lib/sessions/index-subscriptions.test.ts index 066a1831a035..cb39c4fc645b 100644 --- a/ui/src/lib/sessions/index-subscriptions.test.ts +++ b/ui/src/lib/sessions/index-subscriptions.test.ts @@ -264,6 +264,8 @@ describe("createSessionCapability message subscriptions", () => { const client = { request, forceReconnect } as unknown as GatewayBrowserClient; let current = true; const operations = createSessionScopedOperations({ + notifyCreated: vi.fn(), + reportError: vi.fn(), connection: { capture: () => ({ client, epoch: 0 }), isCurrent: () => current, diff --git a/ui/src/lib/sessions/index.create-background.test.ts b/ui/src/lib/sessions/index.create-background.test.ts index 9020e5fe6104..8df5e794681a 100644 --- a/ui/src/lib/sessions/index.create-background.test.ts +++ b/ui/src/lib/sessions/index.create-background.test.ts @@ -4,66 +4,105 @@ import type { SessionsListResult } from "../../api/types.ts"; import { waitForFast } from "../../test-helpers/wait-for.ts"; import { createSessionCapability } from "./index.ts"; -it("claims created placement while carrying work metadata through background reconciliation", async () => { - let resolveList: (result: SessionsListResult) => void = () => undefined; - const pendingList = new Promise((resolve) => { - resolveList = resolve; - }); - const key = "agent:main:created-in-background"; - const request = vi.fn(async (method: string) => { - if (method === "sessions.create") { - return { key }; - } - if (method === "sessions.list") { - return await pendingList; - } - throw new Error(`Unexpected request: ${method}`); - }); - const client = { request } as unknown as GatewayBrowserClient; - const sessions = createSessionCapability({ - snapshot: { - client, - phase: "connected" as const, - hello: null, - assistantAgentId: "main", - sessionKey: "agent:main:main", - }, - subscribe: () => () => undefined, - subscribeEvents: () => () => undefined, - }); - const created = vi.fn(); - sessions.subscribeCreated(created); - - await expect( - sessions.createResult( - { agentId: "main", model: "openai/gpt-5.6-sol", worktree: true }, - { reconciliation: "background" }, - ), - ).resolves.toMatchObject({ key }); - expect(created).toHaveBeenCalledOnce(); - expect(created).toHaveBeenCalledWith(key); - expect(sessions.isPreparedWorkSession(key)).toBe(true); - expect(sessions.state.modelOverrides[key]).toBe("openai/gpt-5.6-sol"); - - resolveList({ - ts: 2, - path: "(multiple)", - count: 1, - defaults: { modelProvider: null, model: null, contextTokens: null }, - sessions: [ - { - key, - kind: "direct", - updatedAt: 2, - worktree: { id: "wt-1", branch: "openclaw/task", repoRoot: "/repo" }, +it.each(["none", "newer", "failed-before-list", "failed-after-list"])( + "reconciles created placement without retiring a newer model claim (%s)", + async (claim) => { + let resolveList: (result: SessionsListResult) => void = () => undefined; + const pendingList = new Promise((resolve) => { + resolveList = resolve; + }); + const key = "agent:main:created-in-background"; + let rejectPatch: (error: Error) => void = () => undefined; + let resolvePatch: (result: unknown) => void = () => undefined; + const pendingPatch = new Promise((resolve, reject) => { + resolvePatch = resolve; + rejectPatch = reject; + }); + const request = vi.fn(async (method: string) => { + if (method === "sessions.create") { + return { key }; + } + if (method === "sessions.list") { + return await pendingList; + } + if (method === "sessions.patch") { + return await pendingPatch; + } + throw new Error(`Unexpected request: ${method}`); + }); + const client = { request } as unknown as GatewayBrowserClient; + const sessions = createSessionCapability({ + snapshot: { + client, + phase: "connected" as const, + hello: null, + assistantAgentId: "main", + sessionKey: "agent:main:main", }, - ], - }); - await waitForFast(() => expect(sessions.isPreparedWorkSession(key)).toBe(false)); - expect(created).toHaveBeenCalledOnce(); - expect(sessions.isPreparedWorkSession(key)).toBe(false); - sessions.dispose(); -}); + subscribe: () => () => undefined, + subscribeEvents: () => () => undefined, + }); + const created = vi.fn(); + sessions.subscribeCreated(created); + + await expect( + sessions.createResult( + { agentId: "main", model: "openai/gpt-5.6-sol", worktree: true }, + { reconciliation: "background" }, + ), + ).resolves.toMatchObject({ key }); + expect(created).toHaveBeenCalledOnce(); + expect(created).toHaveBeenCalledWith(key); + expect(sessions.isPreparedWorkSession(key)).toBe(true); + expect(sessions.state.modelOverrides[key]).toBe("openai/gpt-5.6-sol"); + const patch = + claim !== "none" + ? sessions + .patch(key, { model: claim === "newer" ? "openai/gpt-5.6-sol" : "openai/gpt-5-mini" }) + .catch((error: unknown) => error) + : null; + if (claim === "failed-before-list") { + rejectPatch(new Error("model rejected")); + expect(await patch).toEqual(new Error("model rejected")); + expect(sessions.state.modelOverrides[key]).toBe("openai/gpt-5.6-sol"); + } + + resolveList({ + ts: 2, + path: "(multiple)", + count: 1, + defaults: { modelProvider: null, model: null, contextTokens: null }, + sessions: [ + { + key, + kind: "direct", + updatedAt: 2, + model: "gpt-5.6-sol", + modelProvider: "openai", + modelOverrideSource: null, + worktree: { id: "wt-1", branch: "openclaw/task", repoRoot: "/repo" }, + }, + ], + }); + await waitForFast(() => expect(sessions.isPreparedWorkSession(key)).toBe(false)); + if (claim === "failed-after-list") { + expect(sessions.state.modelOverrides[key]).toBe("openai/gpt-5-mini"); + rejectPatch(new Error("model rejected")); + expect(await patch).toEqual(new Error("model rejected")); + } + expect(created).toHaveBeenCalledOnce(); + expect(sessions.isPreparedWorkSession(key)).toBe(false); + expect(sessions.state.modelOverrides[key]).toBe( + claim === "newer" ? "openai/gpt-5.6-sol" : undefined, + ); + if (claim === "newer") { + resolvePatch({ ok: true, key, entry: {} }); + await patch; + expect(sessions.state.modelOverrides[key]).toBeUndefined(); + } + sessions.dispose(); + }, +); it("retires prepared work placement when the session is deleted", async () => { const key = "agent:main:deleted-worktree"; diff --git a/ui/src/lib/sessions/index.recovery.test.ts b/ui/src/lib/sessions/index.recovery.test.ts new file mode 100644 index 000000000000..0ccba9c074b4 --- /dev/null +++ b/ui/src/lib/sessions/index.recovery.test.ts @@ -0,0 +1,59 @@ +import { expect, it, vi } from "vitest"; +import { createDeferred } from "../../../../test/helpers/promise.js"; +import type { GatewayBrowserClient } from "../../api/gateway.ts"; +import { waitForFast } from "../../test-helpers/wait-for.ts"; +import { createSessionCapability } from "./index.ts"; +import { createGatewayHarness, sessionsResult } from "./session-capability.test-support.ts"; + +it.each(["success", "failure", "replaced", "replaced-during-refresh"])( + "keeps recovery notifications, errors and refresh scoped to the connection (%s)", + async (outcome) => { + const recovery = createDeferred(); + const list = createDeferred(); + const request = vi.fn(async (method: string) => { + if (method === "sessions.recover") { + return await recovery.promise; + } + if (method === "sessions.list") { + return await list.promise; + } + throw new Error(`Unexpected request: ${method}`); + }); + const { gateway, publish } = createGatewayHarness({ + request, + } as unknown as GatewayBrowserClient); + const sessions = createSessionCapability(gateway); + const created = vi.fn(); + sessions.subscribeCreated(created); + const operation = sessions.recover({ key: "agent:main:expired", agentId: "main" }); + const successor = { ok: true, key: "agent:main:recovered", sessionId: "successor" }; + + if (outcome === "replaced") { + publish(false); + } + if (outcome === "failure") { + recovery.reject(new Error("recovery rejected")); + } else { + recovery.resolve(successor); + } + if (outcome === "success" || outcome === "replaced-during-refresh") { + await waitForFast(() => expect(created).toHaveBeenCalledWith(successor.key)); + expect(request).toHaveBeenCalledWith( + "sessions.list", + expect.objectContaining({ agentId: "main" }), + ); + if (outcome === "replaced-during-refresh") { + publish(false); + } + list.resolve(sessionsResult([{ key: successor.key, kind: "direct", updatedAt: 1 }], 1)); + } + + await expect(operation).resolves.toEqual(outcome === "success" ? successor : null); + expect(sessions.state.error).toBe(outcome === "failure" ? "recovery rejected" : null); + if (outcome === "failure" || outcome === "replaced") { + expect(created).not.toHaveBeenCalled(); + expect(request).not.toHaveBeenCalledWith("sessions.list", expect.anything()); + } + sessions.dispose(); + }, +); diff --git a/ui/src/lib/sessions/index.ts b/ui/src/lib/sessions/index.ts index 5e9792da66db..0e153b8a7c20 100644 --- a/ui/src/lib/sessions/index.ts +++ b/ui/src/lib/sessions/index.ts @@ -1,6 +1,7 @@ import type { SessionCatalogPullRequestSummary } from "../../../../packages/gateway-protocol/src/schema/sessions-catalog.js"; import { GatewayRequestError, type GatewayEventFrame } from "../../api/gateway.ts"; import type { GatewaySessionRow, SessionsListResult } from "../../api/types.ts"; +import { formatUiError } from "../format-error.ts"; import { createGatewayConnectionLifecycle } from "../gateway-connection-lifecycle.ts"; import { scopedAgentListParamsForSession } from "./navigation.ts"; import { @@ -178,6 +179,12 @@ export function createSessionCapability(gateway: SessionGateway): SessionCapabil retryDelayMs: sessionRetryDelayMs, }); + const notifyCreated = (key: string) => { + for (const listener of createdListeners) { + listener(key); + } + }; + const mutations = createSessionMutations({ connection, readState: () => state, @@ -185,11 +192,7 @@ export function createSessionCapability(gateway: SessionGateway): SessionCapabil refreshReplacement: (agentId) => roster.refreshReplacement(agentId), publishedRow: (key) => roster.publishedRow(key), redecorateLists: () => roster.redecorateLists(), - notifyCreated(key) { - for (const listener of createdListeners) { - listener(key); - } - }, + notifyCreated, retirePullRequestSummary, }); @@ -197,6 +200,8 @@ export function createSessionCapability(gateway: SessionGateway): SessionCapabil connection, agentId: () => state.agentId, refreshReplacement: (agentId) => roster.refreshReplacement(agentId), + notifyCreated, + reportError: (error) => publish({ ...state, error: formatUiError(error) }, "operation"), }); const pullRequestSummary = (key: string) => pullRequestSummaries.get(key.trim()); @@ -527,13 +532,12 @@ export function createSessionCapability(gateway: SessionGateway): SessionCapabil refreshReplacement: roster.refreshReplacement, createResult: mutations.createResult, create: mutations.create, - recover: mutations.recover, + recover: operations.recover, patch: mutations.patch, archiveVisibility: mutations.archiveVisibility, setArchiveVisibility: mutations.setArchiveVisibility, assignOwner: mutations.assignOwner, retireModelOverride: mutations.retireModelOverride, - setModelOverride: mutations.setModelOverride, patchRowLocal: mutations.patchRowLocal, isPreparedWorkSession: mutations.isPreparedWorkSession, pullRequestSummary, diff --git a/ui/src/lib/sessions/list-options.test.ts b/ui/src/lib/sessions/list-options.test.ts index 145247969864..c3bb1ba8dd6b 100644 --- a/ui/src/lib/sessions/list-options.test.ts +++ b/ui/src/lib/sessions/list-options.test.ts @@ -692,7 +692,7 @@ describe("session list replacement options", () => { sessions.dispose(); }); - it("defers model override publication when the caller owns lifecycle validation", async () => { + it("does not publish a model override when the captured UI owner is already retired", async () => { const pendingPatch = deferred(); const request = vi.fn(async (method: string) => { if (method === "sessions.patch") { @@ -702,18 +702,17 @@ describe("session list replacement options", () => { }); const key = "global"; const sessions = createSessions({ request } as unknown as GatewayBrowserClient, key); - sessions.setModelOverride(key, "openai/gpt-old"); const operation = sessions.patch( key, { model: "openai/gpt-new" }, - { deferListRefresh: true, deferModelOverride: true }, + { deferListRefresh: true, ownsModelOverride: () => false }, ); - expect(sessions.state.modelOverrides[key]).toBe("openai/gpt-old"); + expect(sessions.state.modelOverrides[key]).toBeUndefined(); pendingPatch.resolve({ ok: true, path: "", key, entry: {} }); await expect(operation).resolves.toMatchObject({ ok: true, key }); - expect(sessions.state.modelOverrides[key]).toBe("openai/gpt-old"); + expect(sessions.state.modelOverrides[key]).toBeUndefined(); sessions.dispose(); }); @@ -730,7 +729,6 @@ describe("session list replacement options", () => { 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, @@ -757,13 +755,20 @@ describe("session list replacement options", () => { }, ); - it.each(["resolve", "reject"] as const)( - "preserves a replacement owner's equal-value model claim when an older request %s", - async (outcome) => { + it.each([ + ["resolve", false], + ["reject", false], + ["resolve", true], + ["reject", true], + ] as const)( + "preserves a newer equal-value model claim when an older request %s (owner active: %s)", + async (outcome, ownerActive) => { const pendingPatch = deferred(); + const replacementPatch = deferred(); + let patchCount = 0; const request = vi.fn(async (method: string) => { if (method === "sessions.patch") { - return await pendingPatch.promise; + return await (++patchCount === 1 ? pendingPatch.promise : replacementPatch.promise); } throw new Error(`Unexpected request: ${method}`); }); @@ -781,8 +786,12 @@ describe("session list replacement options", () => { ); expect(sessions.state.modelOverrides[key]).toBe("openai/gpt-shared"); - ownsModelOverride = false; - sessions.setModelOverride(key, "openai/gpt-shared"); + ownsModelOverride = ownerActive; + const replacement = sessions.patch( + key, + { model: "openai/gpt-shared" }, + { deferListRefresh: true }, + ); if (outcome === "resolve") { pendingPatch.resolve({ ok: true, path: "", key, entry: {} }); await expect(operation).resolves.toMatchObject({ ok: true, key }); @@ -792,7 +801,11 @@ describe("session list replacement options", () => { } expect(sessions.state.modelOverrides[key]).toBe("openai/gpt-shared"); - expect(sessions.state.error).toBeNull(); + expect(sessions.state.error).toBe( + ownerActive && outcome === "reject" ? "agent A patch failed" : null, + ); + replacementPatch.resolve({ ok: true, key, entry: {} }); + await replacement; sessions.dispose(); }, ); @@ -810,7 +823,6 @@ describe("session list replacement options", () => { }); const key = "global"; const sessions = createSessions({ request } as unknown as GatewayBrowserClient, key); - sessions.setModelOverride(key, "openai/gpt-agent-a-old"); const agentAOperation = sessions.patch( key, diff --git a/ui/src/lib/sessions/patch.ts b/ui/src/lib/sessions/patch.ts index 4ca23d380644..8921d896cfa3 100644 --- a/ui/src/lib/sessions/patch.ts +++ b/ui/src/lib/sessions/patch.ts @@ -32,8 +32,6 @@ export type SessionPatchOptions = { expectedSessionId?: string; /** Explicit unread marker observed by an automatic read acknowledgement. */ expectedMarkedUnreadAt?: number | null; - /** 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. */ diff --git a/ui/src/lib/sessions/reconcile.test.ts b/ui/src/lib/sessions/reconcile.test.ts index 469bb3d9bb4c..549bfa307ca3 100644 --- a/ui/src/lib/sessions/reconcile.test.ts +++ b/ui/src/lib/sessions/reconcile.test.ts @@ -142,6 +142,7 @@ test("sessions.changed deletes every nested null tombstone, not a hand-kept list controlOwnerSessionKey: "agent:main:owner", restartRecoveryStatus: "pending", goal: "ship it", + modelOverrideSource: "user", } as never, ], }; @@ -159,6 +160,7 @@ test("sessions.changed deletes every nested null tombstone, not a hand-kept list controlOwnerSessionKey: null, restartRecoveryStatus: null, goal: null, + modelOverrideSource: null, }, } as never); @@ -176,6 +178,10 @@ test("sessions.changed deletes every nested null tombstone, not a hand-kept list } // updatedAt stays legitimately nullable and must not be deleted by the loop. expect(row?.updatedAt).toBe(2); + // Clearing a pin means the gateway confirmed inheritance. Deleting that null would + // make the row indistinguishable from a gateway too old to report provenance, and + // the picker would keep showing the cleared pin. + expect(row?.modelOverrideSource).toBeNull(); }); test("sessions.changed clears exact run ids only for an explicit tombstone", () => { diff --git a/ui/src/lib/sessions/reconcile.ts b/ui/src/lib/sessions/reconcile.ts index ce532de361db..2d4b989a24d7 100644 --- a/ui/src/lib/sessions/reconcile.ts +++ b/ui/src/lib/sessions/reconcile.ts @@ -361,6 +361,13 @@ export function readSessionChangedEvent(payload: unknown): SessionChangedEventIn }; } +// Null source confirms inheritance; omission on a lifecycle event preserves selection. +const NULLABLE_SESSION_ROW_FIELDS = new Set([ + "updatedAt", + "activeLeafEntryId", + "modelOverrideSource", +]); + export function reconcileSessionChanged( result: SessionsListResult | null, payload: unknown, @@ -483,10 +490,10 @@ export function reconcileSessionChanged( // typed optional-not-null, so every null tombstone deletes — a hand-kept // field list here drifts as new tombstoned fields ship (it already had: // toolOverrides/observerDigest/controlOwnerSessionKey/restartRecoveryStatus/ - // goal leaked null). updatedAt/activeLeafEntryId are the schema's only - // legitimately nullable row fields and keep their explicit handling. + // goal leaked null). Only the fields below are legitimately nullable in the + // schema, where null is the value itself rather than a clear instruction. for (const [field, value] of Object.entries(rowFields)) { - if (value === null && field !== "updatedAt" && field !== "activeLeafEntryId") { + if (value === null && !NULLABLE_SESSION_ROW_FIELDS.has(field)) { delete row[field as keyof GatewaySessionRow]; } } diff --git a/ui/src/lib/sessions/session-capability.ts b/ui/src/lib/sessions/session-capability.ts index 4177a0521b9a..1addb2009cca 100644 --- a/ui/src/lib/sessions/session-capability.ts +++ b/ui/src/lib/sessions/session-capability.ts @@ -187,7 +187,6 @@ export type SessionCapability = { owner: SessionsAssignOwnerParams["owner"], options?: { agentId?: string | null }, ) => Promise; - setModelOverride: (key: string, value: string | null | undefined) => void; retireModelOverride: (key: string) => void; /** Keep optimistic row changes in the published snapshot through later publishes. */ patchRowLocal: (key: string, patch: Partial) => void; diff --git a/ui/src/lib/sessions/session-model-override.test.ts b/ui/src/lib/sessions/session-model-override.test.ts index e847829ee4d3..da67ca8a82b1 100644 --- a/ui/src/lib/sessions/session-model-override.test.ts +++ b/ui/src/lib/sessions/session-model-override.test.ts @@ -8,6 +8,47 @@ import { createSessionCapability } from "./index.ts"; import { createGatewayHarness, sessionsResult } from "./session-capability.test-support.ts"; describe("session model override lifecycle", () => { + it.each(["resolve", "reject"])( + "does not resurrect a deleted session's model when its pending patch %s", + async (outcome) => { + const pending = createDeferred(); + const key = "agent:main:main"; + let patchCount = 0; + const request = vi.fn(async (method: string) => { + if (method === "sessions.patch") { + return ++patchCount === 1 ? { ok: true, key, entry: {} } : pending.promise; + } + if (method === "sessions.delete") { + return { deleted: true }; + } + if (method === "sessions.list") { + return sessionsResult([], 2); + } + throw new Error(`Unexpected request: ${method}`); + }); + const { gateway } = createGatewayHarness({ request } as unknown as GatewayBrowserClient); + const sessions = createSessionCapability(gateway); + await sessions.patch(key, { model: "openai/gpt-old" }, { deferListRefresh: true }); + const operation = sessions.patch( + key, + { model: "openai/gpt-new" }, + { deferListRefresh: true }, + ); + expect(sessions.state.modelOverrides[key]).toBe("openai/gpt-new"); + await expect(sessions.delete(key)).resolves.toMatchObject({ deleted: true }); + expect(sessions.state.modelOverrides).toEqual({}); + if (outcome === "resolve") { + pending.resolve({ ok: true, key, entry: {} }); + await operation; + } else { + pending.reject(new Error("model rejected")); + await expect(operation).rejects.toThrow("model rejected"); + } + expect(sessions.state.modelOverrides).toEqual({}); + sessions.dispose(); + }, + ); + it("retains a confirmed patch without publishing its retired optimistic state", async () => { const stalePatch = createDeferred(); const request = vi.fn(async (method: string) => { @@ -27,8 +68,7 @@ describe("session model override lifecycle", () => { const sessions = createSessionCapability(gateway); const key = "agent:main:main"; const inactiveKey = "agent:main:inactive"; - sessions.setModelOverride(key, "openai/gpt-old"); - sessions.setModelOverride(inactiveKey, "openai/gpt-old-account"); + const inactiveOperation = sessions.patch(inactiveKey, { model: "openai/gpt-old-account" }); const operation = sessions.patch(key, { model: "openai/gpt-new" }); expect(sessions.state.modelOverrides[key]).toBe("openai/gpt-new"); @@ -39,6 +79,7 @@ describe("session model override lifecycle", () => { stalePatch.resolve({ ok: true, path: "", key, entry: {} }); await expect(operation).resolves.toMatchObject({ ok: true, key }); + await inactiveOperation; expect(sessions.state.modelOverrides).toEqual({}); expect(sessions.state.error).toContain("completed on the previous connection"); expect( @@ -98,14 +139,13 @@ describe("session model override lifecycle", () => { const { gateway, publish } = createGatewayHarness(client); const sessions = createSessionCapability(gateway); const key = "agent:main:main"; - sessions.setModelOverride(key, "openai/gpt-old"); const operation = sessions.patch( key, { model: "openai/gpt-new" }, { waitFor: priorPatch.promise }, ); - expect(sessions.state.modelOverrides[key]).toBe("openai/gpt-old"); + expect(sessions.state.modelOverrides[key]).toBeUndefined(); expect(request).not.toHaveBeenCalledWith("sessions.patch", expect.anything()); publish(false); diff --git a/ui/src/lib/sessions/session-mutations.ts b/ui/src/lib/sessions/session-mutations.ts index 93e35f2849ce..c82c3456b7f4 100644 --- a/ui/src/lib/sessions/session-mutations.ts +++ b/ui/src/lib/sessions/session-mutations.ts @@ -16,7 +16,6 @@ import { type SessionCreateParams, } from "./create.ts"; import type { SessionPatch, SessionPatchOptions } from "./patch.ts"; -import { requestSessionRecovery } from "./recover.ts"; import { createSessionArchiveVisibility } from "./session-archive-visibility.ts"; import type { SessionConnectionOwner, @@ -57,7 +56,11 @@ type SessionMutationsHost = { export function createSessionMutations(host: SessionMutationsHost) { const pendingModelPatches = new Map< string, - { token: symbol; previous: string | null | undefined; revision: number } + { + token: symbol; + previous: { value: string | null | undefined; created: boolean }; + revision: number; + } >(); const pendingPinPatches = new Map< string, @@ -68,12 +71,19 @@ export function createSessionMutations(host: SessionMutationsHost) { host.publish({ ...host.readState() }), ); const preparedWorkSessionKeys = new Set(); + const pendingCreatedModelOverrides = new Set(); - const setModelOverride = (key: string, value: string | null | undefined) => { + const setModelOverride = (key: string, value: string | null | undefined, created = false) => { const normalizedKey = key.trim(); if (!normalizedKey) { return; } + // Register before publishing: a synchronous subscriber may claim the same value. + if (created) { + pendingCreatedModelOverrides.add(normalizedKey); + } else { + pendingCreatedModelOverrides.delete(normalizedKey); + } // Equal-value writes still transfer ownership while a patch is pending. const pendingModelPatch = pendingModelPatches.get(normalizedKey); if (pendingModelPatch) { @@ -191,7 +201,7 @@ export function createSessionMutations(host: SessionMutationsHost) { preparedWorkSessionKeys.add(result.key.trim()); } if (requestParams.model?.trim()) { - setModelOverride(result.key, requestParams.model); + setModelOverride(result.key, requestParams.model, true); } else if (preparedWorkSessionKeys.has(result.key)) { host.publish({ ...host.readState() }); } @@ -222,27 +232,6 @@ export function createSessionMutations(host: SessionMutationsHost) { const create = async (params: SessionCreateParams = {}) => (await createResult(params))?.key ?? null; - const recover = async (params: { key: string; agentId?: string }) => { - const scope = host.connection.capture(); - if (!scope) { - return null; - } - try { - const result = await requestSessionRecovery(scope.client, params); - if (!host.connection.isCurrent(scope)) { - return null; - } - host.notifyCreated(result.key); - await host.refreshReplacement(params.agentId); - return host.connection.isCurrent(scope) ? result : null; - } catch (error) { - if (host.connection.isCurrent(scope)) { - host.publish({ ...host.readState(), error: formatUiError(error) }, "operation"); - } - return null; - } - }; - const patch = async ( key: string, patchParams: SessionPatch, @@ -252,12 +241,10 @@ export function createSessionMutations(host: SessionMutationsHost) { if (!scope) { return null; } - const hasModelPatch = Object.hasOwn(patchParams, "model"); - const managesModelOverride = hasModelPatch && options.deferModelOverride !== true; + const managesModelOverride = Object.hasOwn(patchParams, "model"); const normalizedKey = key.trim(); const archivedPresentationRow = patchParams.archived === true ? host.publishedRow(normalizedKey) : undefined; - let previousModelOverride: string | null | undefined; let modelPatchStarted = false; let modelPatchRevision = 0; const modelPatchToken = Symbol("session-model-patch"); @@ -267,13 +254,13 @@ export function createSessionMutations(host: SessionMutationsHost) { return; } const pendingModelPatch = pendingModelPatches.get(normalizedKey); - previousModelOverride = pendingModelPatch - ? pendingModelPatch.previous - : host.readState().modelOverrides[normalizedKey]; modelPatchStarted = true; pendingModelPatches.set(normalizedKey, { token: modelPatchToken, - previous: previousModelOverride, + previous: pendingModelPatch?.previous ?? { + value: host.readState().modelOverrides[normalizedKey], + created: pendingCreatedModelOverrides.has(normalizedKey), + }, revision: 0, }); setModelOverride(key, patchParams.model); @@ -316,21 +303,33 @@ export function createSessionMutations(host: SessionMutationsHost) { const pendingModelPatch = pendingModelPatches.get(normalizedKey); if (modelPatchStarted && pendingModelPatch?.token === modelPatchToken) { pendingModelPatches.delete(normalizedKey); + // Success and rollback may settle only this operation's untouched claim. + if (pendingModelPatch.revision !== modelPatchRevision) { + return; + } if (host.connection.isCurrent(scope) && ownsModelOverride()) { if (completed && !options.deferListRefresh) { // The refreshed row already carries the Gateway-confirmed selection. - // Retiring the local override (instead of re-asserting it forever) - // lets external model changes — another window, a channel /model, - // a fallback rotation — reach this window; a retained entry would - // shadow the server row for the connection lifetime. Untouched only - // when a newer claim wrote the key while this patch was in flight. - if (pendingModelPatch.revision === modelPatchRevision) { - setModelOverride(key, undefined); - } + // Keeping an overlay would hide subsequent external model changes. + setModelOverride(key, undefined); } else { - setModelOverride(key, completed ? patchParams.model : previousModelOverride); + const previous = pendingModelPatch.previous; + // A failed patch restores a create preview only until its canonical row arrives. + const created = + !completed && + previous.created && + host.publishedRow(normalizedKey)?.modelOverrideSource === undefined; + setModelOverride( + key, + completed + ? patchParams.model + : previous.created && !created + ? undefined + : previous.value, + created, + ); } - } else if (pendingModelPatch.revision === modelPatchRevision) { + } else { // The shared key now belongs to another agent/connection. Remove only // this operation's untouched optimistic value; preserve newer claims. setModelOverride(key, undefined); @@ -627,7 +626,6 @@ export function createSessionMutations(host: SessionMutationsHost) { return { create, createResult, - recover, delete: remove, deleteMany: removeMany, patch, @@ -718,7 +716,6 @@ export function createSessionMutations(host: SessionMutationsHost) { }, reset, retireModelOverride, - setModelOverride, archiveVisibility: archiveVisibility.get, setArchiveVisibility: (key: string, visibility: SessionArchiveVisibility | undefined) => archiveVisibility.set(key, visibility), @@ -726,12 +723,16 @@ export function createSessionMutations(host: SessionMutationsHost) { settlePrepared(result: SessionsListResult | null) { archiveVisibility.settle(result); for (const row of result?.sessions ?? []) { + if (row.modelOverrideSource !== undefined && pendingCreatedModelOverrides.has(row.key)) { + setModelOverride(row.key, undefined); + } if (row.worktree || row.execNode) { preparedWorkSessionKeys.delete(row.key); } } }, retireConnection() { + pendingCreatedModelOverrides.clear(); pendingModelPatches.clear(); // Pin intents live inside `result`, which the replacement connection // rehydrates wholesale; only the model-override side map outlives that @@ -746,6 +747,7 @@ export function createSessionMutations(host: SessionMutationsHost) { } }, dispose() { + pendingCreatedModelOverrides.clear(); pendingModelPatches.clear(); pendingPinPatches.clear(); confirmedArchives.clear(); diff --git a/ui/src/lib/sessions/session-scoped-operations.ts b/ui/src/lib/sessions/session-scoped-operations.ts index 0572ebfdfbea..740b68c0c9ed 100644 --- a/ui/src/lib/sessions/session-scoped-operations.ts +++ b/ui/src/lib/sessions/session-scoped-operations.ts @@ -17,6 +17,7 @@ import type { SessionWorkspaceListResult, SessionWorkspaceSetResult, } from "../../api/types.ts"; +import { requestSessionRecovery } from "./recover.ts"; import type { SessionCompactResult, SessionConnectionOwner, @@ -46,6 +47,8 @@ type SessionScopedOperationsHost = { connection: SessionConnectionOwner; agentId: () => string | null; refreshReplacement: (agentId?: string | null) => Promise; + notifyCreated: (key: string) => void; + reportError: (error: unknown) => void; }; const retiredFailedSubscriptionRecoveries = new WeakSet(); @@ -53,6 +56,27 @@ const retiredFailedSubscriptionRecoveries = new WeakSet(); export function createSessionScopedOperations(host: SessionScopedOperationsHost) { const ownedSubscriptions = new Set(); + const recover = async (params: { key: string; agentId?: string }) => { + const scope = host.connection.capture(); + if (!scope) { + return null; + } + try { + const result = await requestSessionRecovery(scope.client, params); + if (!host.connection.isCurrent(scope)) { + return null; + } + host.notifyCreated(result.key); + await host.refreshReplacement(params.agentId); + return host.connection.isCurrent(scope) ? result : null; + } catch (error) { + if (host.connection.isCurrent(scope)) { + host.reportError(error); + } + return null; + } + }; + const compact = async ( key: string, options: { agentId?: string | null } = {}, @@ -280,6 +304,7 @@ export function createSessionScopedOperations(host: SessionScopedOperationsHost) listBranches, listCheckpoints, listFiles, + recover, restoreCheckpoint, rewind, setFile, diff --git a/ui/src/pages/chat/chat-command-executor.test.ts b/ui/src/pages/chat/chat-command-executor.test.ts index a22846b66875..237aa0f1da62 100644 --- a/ui/src/pages/chat/chat-command-executor.test.ts +++ b/ui/src/pages/chat/chat-command-executor.test.ts @@ -12,7 +12,6 @@ import { createSessionCapability, type SessionCapability } from "../../lib/sessi import { createResolvedModelPatch, createModelCatalog, - DEEPSEEK_CHAT_MODEL, OPENAI_GPT5_MINI_MODEL, } from "../../test-helpers/chat-model.ts"; import { createTestGatewayClient } from "../../test-helpers/gateway-client.ts"; @@ -122,6 +121,41 @@ function expectNoRequestCall(request: ReturnType, method: string) } describe("executeSlashCommand directives", () => { + it("lets the canonical row retire a slash-command selection equal to the default", async () => { + const key = "agent:main:main"; + const request = vi.fn(async (method: string) => { + if (method === "sessions.patch") { + return createResolvedModelPatch("gpt-5-mini", "openai"); + } + if (method === "sessions.list") { + return createSessionsResult([ + row(key, { + model: "gpt-5-mini", + modelProvider: "openai", + modelOverrideSource: null, + }), + ]); + } + throw new Error(`Unexpected request: ${method}`); + }); + const client = createTestGatewayClient(request); + const snapshot = { client, phase: "connected" as const, hello: sessionMutationGatewayHello() }; + const sessions = createSessionCapability({ + snapshot, + subscribe: () => () => undefined, + subscribeEvents: () => () => undefined, + }); + const result = await executeSlashCommandImpl(client, key, "model", "openai/gpt-5-mini", { + sessions, + sessionAccessSnapshot: snapshot, + chatModelCatalog: createModelCatalog(OPENAI_GPT5_MINI_MODEL), + }); + expect(result.failed).not.toBe(true); + expect(sessions.state.result?.sessions[0]?.modelOverrideSource).toBeNull(); + expect(sessions.state.modelOverrides).toEqual({}); + sessions.dispose(); + }); + it("does not compact a session without operator.admin", async () => { const request = vi.fn(); const client = createTestGatewayClient(request); @@ -157,19 +191,17 @@ describe("executeSlashCommand directives", () => { } }); - it("defers slash-command model cache publication to the captured chat owner", async () => { + it("passes the captured chat owner to canonical model patching", async () => { const client = createTestGatewayClient(vi.fn()); const patch = vi .fn() .mockResolvedValue( createResolvedModelPatch(OPENAI_GPT5_MINI_MODEL.id, OPENAI_GPT5_MINI_MODEL.provider), ); - const setModelOverride = vi.fn(); const ownsModelOverride = vi.fn(() => true); const sessions = { ...createTestSessionCapability(client), patch, - setModelOverride, } as SessionCapability; const result = await executeSlashCommandImpl(client, "global", "model", "gpt-5-mini", { @@ -190,40 +222,9 @@ describe("executeSlashCommand directives", () => { { model: "gpt-5-mini" }, expect.objectContaining({ agentId: "work", - deferModelOverride: true, ownsModelOverride, }), ); - 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 = createTestGatewayClient(vi.fn()); - const setModelOverride = vi.fn(); - const sessions = { - ...createTestSessionCapability(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: sessionMutationGatewayHello(), - 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 () => { @@ -540,39 +541,31 @@ describe("executeSlashCommand directives", () => { ); }); - it("mirrors resolved provider-qualified model refs after /model changes", async () => { - const request = vi.fn(async (method: string, _payload?: unknown) => { - if (method === "sessions.patch") { - return createResolvedModelPatch("gpt-5-mini", "openai"); - } - if (method === "models.list") { - return { models: createModelCatalog(OPENAI_GPT5_MINI_MODEL) }; - } - if (method === "models.list") { - return { models: [{ id: "gpt-5-mini", name: "gpt-5-mini", provider: "openai" }] }; - } - throw new Error(`unexpected method: ${method}`); - }); + it.each(["gpt-5-mini", "openai/gpt-5-mini", "nvidia/moonshotai/kimi-k2.5"])( + "patches %s without rebuilding a second model cache", + async (model) => { + const request = vi.fn(async (method: string, _payload?: unknown) => { + if (method === "sessions.patch") { + return createResolvedModelPatch("gpt-5-mini", "openai"); + } + throw new Error(`unexpected method: ${method}`); + }); - const result = await executeSlashCommand( - createTestGatewayClient(request), - "main", - "model", - "gpt-5-mini", - { - chatModelCatalog: [{ id: "gpt-5-mini", name: "gpt-5-mini", provider: "openai" }], - }, - ); + const result = await executeSlashCommand( + createTestGatewayClient(request), + "main", + "model", + model, + ); - expect(request).toHaveBeenCalledWith("sessions.patch", { - key: "main", - model: "gpt-5-mini", - }); - expect(result.sessionPatch?.modelOverride).toEqual({ - kind: "qualified", - value: "openai/gpt-5-mini", - }); - }); + expect(request).toHaveBeenCalledWith("sessions.patch", { + key: "main", + model, + }); + expect(result.modelChanged).toBe(true); + expectNoRequestCall(request, "models.list"); + }, + ); it("passes selected-agent scope for global model changes", async () => { const request = vi.fn(async (method: string, _payload?: unknown) => { @@ -639,162 +632,6 @@ describe("executeSlashCommand directives", () => { }); }); - it("uses the local model catalog to qualify raw /model overrides when the patch response omits provider", async () => { - const request = vi.fn(async (method: string, _payload?: unknown) => { - if (method === "sessions.patch") { - return { - ok: true, - key: "main", - resolved: { - model: "gpt-5-mini", - }, - }; - } - throw new Error(`unexpected method: ${method}`); - }); - - const result = await executeSlashCommand( - createTestGatewayClient(request), - "main", - "model", - "gpt-5-mini", - { - chatModelCatalog: [{ id: "gpt-5-mini", name: "GPT-5 Mini", provider: "openai" }], - }, - ); - - expect(result.sessionPatch?.modelOverride).toEqual({ - kind: "qualified", - value: "openai/gpt-5-mini", - }); - }); - - it("corrects stale patched providers with the catalog after /model", async () => { - const request = vi.fn(async (method: string, _payload?: unknown) => { - if (method === "sessions.patch") { - return createResolvedModelPatch("deepseek-chat", "zai"); - } - if (method === "models.list") { - return { models: createModelCatalog(DEEPSEEK_CHAT_MODEL) }; - } - throw new Error(`unexpected method: ${method}`); - }); - - const result = await executeSlashCommand( - createTestGatewayClient(request), - "main", - "model", - "deepseek-chat", - ); - - expect(result.sessionPatch?.modelOverride).toEqual({ - kind: "qualified", - value: "deepseek/deepseek-chat", - }); - }); - - it("keeps openrouter-prefixed refs when patched model ids include slashes", async () => { - const request = vi.fn(async (method: string, _payload?: unknown) => { - if (method === "sessions.patch") { - return createResolvedModelPatch("google/gemma-4-26b-a4b-it", "openrouter"); - } - throw new Error(`unexpected method: ${method}`); - }); - - const result = await executeSlashCommand( - createTestGatewayClient(request), - "main", - "model", - "google/gemma-4-26b-a4b-it", - { - chatModelCatalog: [ - { - id: "google/gemma-4-26b-a4b-it", - name: "Gemma 4 26B", - provider: "openrouter", - }, - ], - }, - ); - - expect(result.sessionPatch?.modelOverride).toEqual({ - kind: "qualified", - value: "openrouter/google/gemma-4-26b-a4b-it", - }); - expect(request).toHaveBeenCalledTimes(1); - }); - - it("falls back to the patched server provider when catalog lookup fails", async () => { - const request = vi.fn(async (method: string, _payload?: unknown) => { - if (method === "sessions.patch") { - return createResolvedModelPatch("gpt-5-mini", "openai"); - } - if (method === "models.list") { - throw new Error("models unavailable"); - } - throw new Error(`unexpected method: ${method}`); - }); - - const result = await executeSlashCommand( - createTestGatewayClient(request), - "main", - "model", - "gpt-5-mini", - ); - - expect(result.sessionPatch?.modelOverride).toEqual({ - kind: "qualified", - value: "openai/gpt-5-mini", - }); - }); - - it("keeps provider-qualified nested ids when the patched catalog lookup fails", async () => { - const request = vi.fn(async (method: string, _payload?: unknown) => { - if (method === "sessions.patch") { - return createResolvedModelPatch("moonshotai/kimi-k2.5", "nvidia"); - } - if (method === "models.list") { - throw new Error("models unavailable"); - } - throw new Error(`unexpected method: ${method}`); - }); - - const result = await executeSlashCommand( - createTestGatewayClient(request), - "main", - "model", - "nvidia/moonshotai/kimi-k2.5", - ); - - expect(result.sessionPatch?.modelOverride).toEqual({ - kind: "qualified", - value: "nvidia/moonshotai/kimi-k2.5", - }); - }); - - it("reuses a provided model catalog for /model updates without refetching", async () => { - const request = vi.fn(async (method: string, _payload?: unknown) => { - if (method === "sessions.patch") { - return createResolvedModelPatch("gpt-5-mini", "openai"); - } - throw new Error(`unexpected method: ${method}`); - }); - - const result = await executeSlashCommand( - createTestGatewayClient(request), - "main", - "model", - "gpt-5-mini", - { modelCatalog: createModelCatalog(OPENAI_GPT5_MINI_MODEL) }, - ); - - expect(result.sessionPatch?.modelOverride).toEqual({ - kind: "qualified", - value: "openai/gpt-5-mini", - }); - expect(request).toHaveBeenCalledTimes(1); - expect(request).not.toHaveBeenCalledWith("models.list", {}); - }); it("resolves the legacy main alias for /usage", async () => { const request = vi.fn(async (method: string, _payload?: unknown) => { if (method === "sessions.list") { diff --git a/ui/src/pages/chat/chat-command-executor.ts b/ui/src/pages/chat/chat-command-executor.ts index c943e7eba708..99a83032bea9 100644 --- a/ui/src/pages/chat/chat-command-executor.ts +++ b/ui/src/pages/chat/chat-command-executor.ts @@ -20,11 +20,6 @@ import { getSlashCommandDescription, SLASH_COMMANDS, } from "../../lib/chat/commands.ts"; -import { - type ChatModelOverride, - createChatModelOverride, - resolvePreferredServerChatModelValue, -} from "../../lib/chat/model-ref.ts"; import { normalizeChatFastModeInput, resolveChatFastModeStatus, @@ -52,10 +47,8 @@ type SlashCommandResult = { content: string; /** Side-effect action the caller should perform after displaying the result. */ action?: "refresh" | "export" | "new-session" | "reset" | "stop" | "clear" | "navigate-usage"; - /** Optional session-level directive changes that the caller should mirror locally. */ - sessionPatch?: { - modelOverride?: ChatModelOverride | null; - }; + /** Model-dependent tools need refreshing after a confirmed selection. */ + modelChanged?: boolean; /** When set, the caller should track this as the active run (enables Abort, blocks concurrent sends). */ trackRunId?: string; /** When set, the caller should surface a visible pending item tied to the current run. */ @@ -304,10 +297,6 @@ async function executeModel( try { const requestedModel = args.trim(); - const resolvedModelCatalog = modelCatalog - ? Promise.resolve(modelCatalog) - : loadModelCatalog(client, agentId, { allowFailure: true }); - let resolvedOverride: ChatModelOverride | null = null; await patchSession( context, sessionKey, @@ -315,37 +304,13 @@ async function executeModel( model: requestedModel, }, { - deferModelOverride: true, ownsModelOverride: context.ownsModelOverride, - 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); - } - }, }, ); return { content: t("chat.commandResults.model.set", { model: `\`${requestedModel}\`` }), action: "refresh", - sessionPatch: { modelOverride: resolvedOverride }, + modelChanged: true, }; } catch (err) { return { @@ -792,23 +757,15 @@ async function loadThinkingCommandState( async function loadModelCatalog( client: GatewayBrowserClient, agentId: string | undefined, - opts?: { allowFailure?: boolean }, ): Promise { if (!agentId) { return []; } - try { - const result = await client.request<{ models: ModelCatalogEntry[] }>("models.list", { - agentId, - view: "configured", - }); - return result?.models ?? []; - } catch (err) { - if (opts?.allowFailure) { - return []; - } - throw err; - } + const result = await client.request<{ models: ModelCatalogEntry[] }>("models.list", { + agentId, + view: "configured", + }); + return result?.models ?? []; } function resolveCommandMessage( diff --git a/ui/src/pages/chat/chat-commands.ts b/ui/src/pages/chat/chat-commands.ts index 2bde553f7778..7dafe2b9fb8b 100644 --- a/ui/src/pages/chat/chat-commands.ts +++ b/ui/src/pages/chat/chat-commands.ts @@ -469,7 +469,7 @@ export async function dispatchChatSlashCommand( ); } - if (result.sessionPatch && "modelOverride" in result.sessionPatch) { + if (result.modelChanged) { if (targetIsCurrent()) { await host.refreshCurrentSessionTools?.(); } diff --git a/ui/src/pages/chat/chat-history.inflight.test.ts b/ui/src/pages/chat/chat-history.inflight.test.ts index 75d7d5a1d6c7..f1060048f1c6 100644 --- a/ui/src/pages/chat/chat-history.inflight.test.ts +++ b/ui/src/pages/chat/chat-history.inflight.test.ts @@ -24,7 +24,7 @@ function createState(result: ChatHistoryResult): TestState { sessionKey: "main", }); const sessions: TestSessions = { - setModelOverride: vi.fn(), + refreshReplacement: vi.fn(async () => undefined), reconcileRunTerminal: vi.fn(), }; return { diff --git a/ui/src/pages/chat/chat-history.test.ts b/ui/src/pages/chat/chat-history.test.ts index 94f4815e1dc4..01036d581445 100644 --- a/ui/src/pages/chat/chat-history.test.ts +++ b/ui/src/pages/chat/chat-history.test.ts @@ -32,7 +32,7 @@ function createState(result: ChatHistoryResult): TestState { requestHandlers: { "chat.history": result }, sessionKey: "main", }); - const sessions: TestSessions = { setModelOverride: vi.fn() }; + const sessions: TestSessions = { refreshReplacement: vi.fn(async () => undefined) }; return { ...host, chatToolMessages: host.chatToolMessages ?? [], @@ -89,7 +89,7 @@ describe("syncSelectedSessionMessageSubscription", () => { state.chatSessionMessageSubscriptionRequestedKey = "agent:main:previous"; state.chatSessionMessageSubscription = { key: "agent:main:previous", agentId: null }; state.sessions = { - setModelOverride: vi.fn(), + refreshReplacement: vi.fn(async () => undefined), subscribeMessages, unsubscribeMessages, }; @@ -130,7 +130,7 @@ describe("syncSelectedSessionMessageSubscription", () => { state.chatSessionMessageSubscription = previous; state.sessionsError = null; state.sessions = { - setModelOverride: vi.fn((_key: string, _value: string | null | undefined) => undefined), + refreshReplacement: vi.fn(async () => undefined), subscribeMessages, unsubscribeMessages, }; @@ -163,7 +163,7 @@ describe("syncSelectedSessionMessageSubscription", () => { state.chatSessionMessageSubscription = previous; state.sessionsError = null; state.sessions = { - setModelOverride: vi.fn((_key: string, _value: string | null | undefined) => undefined), + refreshReplacement: vi.fn(async () => undefined), subscribeMessages, unsubscribeMessages, }; @@ -203,7 +203,7 @@ describe("syncSelectedSessionMessageSubscription", () => { state.chatSessionMessageSubscriptionRequestedKey = null; state.chatSessionMessageSubscription = null; state.sessions = { - setModelOverride: vi.fn((_key: string, _value: string | null | undefined) => undefined), + refreshReplacement: vi.fn(async () => undefined), subscribeMessages, unsubscribeMessages, }; @@ -253,7 +253,7 @@ describe("rewindChatHistory", () => { { mimeType: "image/png", data: "A" }, ], }), - setModelOverride: vi.fn(), + refreshReplacement: vi.fn(async () => undefined), }; cacheChatSessionSnapshot( state.chatMessagesBySession, @@ -316,7 +316,7 @@ describe("rewindChatHistory", () => { state.sessionKey = "agent:main:new-selection"; return { editorText: "source draft" }; }), - setModelOverride: vi.fn(), + refreshReplacement: vi.fn(async () => undefined), }; cacheChatSessionSnapshot( state.chatMessagesBySession, @@ -356,7 +356,7 @@ describe("rewindChatHistory", () => { }); state.sessions = { rewind: vi.fn(() => rewind), - setModelOverride: vi.fn(), + refreshReplacement: vi.fn(async () => undefined), }; const pending = rewindChatHistory(state as never, "user-entry"); @@ -405,7 +405,7 @@ describe("switchChatHistoryBranch", () => { }, ]), switchBranch: vi.fn().mockResolvedValue({}), - setModelOverride: vi.fn(), + refreshReplacement: vi.fn(async () => undefined), }; cacheChatSessionSnapshot( state.chatMessagesBySession, @@ -440,7 +440,10 @@ describe("switchChatHistoryBranch", () => { }; state.chatBranchesSessionKey = state.sessionKey; state.chatBranchesConnectionEpoch = state.connectionEpoch - 1; - state.sessions = { listBranches: vi.fn().mockResolvedValue([]), setModelOverride: vi.fn() }; + state.sessions = { + listBranches: vi.fn().mockResolvedValue([]), + refreshReplacement: vi.fn(async () => undefined), + }; await loadChatHistory(state); @@ -459,7 +462,7 @@ describe("switchChatHistoryBranch", () => { .mockResolvedValue([ { leafEntryId: "tip", headline: "tip", messageCount: 1, active: true }, ]), - setModelOverride: vi.fn(), + refreshReplacement: vi.fn(async () => undefined), }; await loadChatHistory(state); @@ -479,7 +482,10 @@ describe("switchChatHistoryBranch", () => { state.sessionKey = "main"; state.chatBranchesSessionKey = "agent:main:main"; state.chatBranchesConnectionEpoch = state.connectionEpoch; - state.sessions = { listBranches: vi.fn().mockResolvedValue([]), setModelOverride: vi.fn() }; + state.sessions = { + listBranches: vi.fn().mockResolvedValue([]), + refreshReplacement: vi.fn(async () => undefined), + }; await loadChatHistory(state); @@ -510,7 +516,7 @@ describe("switchChatHistoryBranch", () => { state.sessions = { listBranches: vi.fn().mockResolvedValue([]), switchBranch: vi.fn().mockResolvedValue({}), - setModelOverride: vi.fn(), + refreshReplacement: vi.fn(async () => undefined), }; const staleHistory = loadChatHistory(state); @@ -544,7 +550,7 @@ describe("switchChatHistoryBranch", () => { state.sessions = { listBranches: vi.fn().mockResolvedValue([]), switchBranch: vi.fn(() => switched), - setModelOverride: vi.fn(), + refreshReplacement: vi.fn(async () => undefined), }; const pending = switchChatHistoryBranch(state as never, "stale-leaf"); diff --git a/ui/src/pages/chat/chat-send.test.ts b/ui/src/pages/chat/chat-send.test.ts index 2d9879cbe643..f4c19521153b 100644 --- a/ui/src/pages/chat/chat-send.test.ts +++ b/ui/src/pages/chat/chat-send.test.ts @@ -2124,11 +2124,18 @@ describe("handleSendChat", () => { expect(patchCount).toBe(2); }); - it("rolls a failed queued picker back to the preceding slash model value", async () => { + it("retires a failed queued picker back to the preceding confirmed session row", async () => { const firstPatch = createDeferred(); let patchCount = 0; const host = makeChatHost({ requestHandlers: { + "sessions.list": createSessionsResult([ + row("agent:main", { + model: "gpt-5-mini", + modelProvider: "openai", + modelOverrideSource: "user", + }), + ]), "sessions.patch": () => { patchCount += 1; if (patchCount === 1) { @@ -2138,29 +2145,23 @@ describe("handleSendChat", () => { }, }, }); - host.sessions.setModelOverride(host.sessionKey, "openai/gpt-old"); - const slash = patchChatSessionSettings( - host, - host.sessionKey, - { model: "openai/gpt-5-mini" }, - { - deferModelOverride: true, - reconcile: () => { - host.sessions.setModelOverride(host.sessionKey, "openai/gpt-5-mini"); - }, - }, - ); + const slash = patchChatSessionSettings(host, host.sessionKey, { model: "openai/gpt-5-mini" }); await waitForFast(() => expect(patchCount).toBe(1)); const picker = patchChatSessionSettings(host, host.sessionKey, { model: "openai/gpt-5", }); - expect(host.sessions.state.modelOverrides[host.sessionKey]).toBe("openai/gpt-old"); + expect(host.sessions.state.modelOverrides[host.sessionKey]).toBe("openai/gpt-5-mini"); firstPatch.resolve(createResolvedModelPatch("gpt-5-mini", "openai")); await expect(slash).resolves.toBeTruthy(); await expect(picker).rejects.toThrow("picker rejected"); - expect(host.sessions.state.modelOverrides[host.sessionKey]).toBe("openai/gpt-5-mini"); + expect(host.sessions.state.modelOverrides[host.sessionKey]).toBeUndefined(); + expect(host.sessions.state.result?.sessions[0]).toMatchObject({ + model: "gpt-5-mini", + modelProvider: "openai", + modelOverrideSource: "user", + }); }); it("keeps waiting when a late picker barrier cannot be persisted", async () => { @@ -3384,7 +3385,7 @@ describe("handleSendChat", () => { await foreground; }); - it("keeps slash-command model changes in sync with the chat header cache", async () => { + it("keeps slash-command model changes in the canonical row and refreshes tools", async () => { vi.stubGlobal( "fetch", vi.fn().mockResolvedValue(createJsonResponse({}, { ok: false })), @@ -3405,9 +3406,15 @@ describe("handleSendChat", () => { "sessions.list": { ts: 0, path: "", - count: 0, + count: 1, defaults: { modelProvider: "openai", model: "gpt-5", contextTokens: null }, - sessions: [], + sessions: [ + row("main", { + model: "gpt-5-mini", + modelProvider: "openai", + modelOverrideSource: "user", + }), + ], }, "models.list": { models: [{ id: "gpt-5-mini", name: "GPT-5 Mini", provider: "openai" }], @@ -3424,7 +3431,12 @@ describe("handleSendChat", () => { key: "main", model: "gpt-5-mini", }); - expect(host.sessions.state.modelOverrides.main).toBe("openai/gpt-5-mini"); + expect(host.sessions.state.modelOverrides.main).toBeUndefined(); + expect(host.sessions.state.result?.sessions[0]).toMatchObject({ + model: "gpt-5-mini", + modelProvider: "openai", + modelOverrideSource: "user", + }); expect(refreshCurrentSessionTools).toHaveBeenCalledTimes(1); }); @@ -5060,9 +5072,7 @@ describe("handleSendChat", () => { action: "refresh", content: "Model set to `gpt-5-mini`.", pendingCurrentRun: true, - sessionPatch: { - modelOverride: { kind: "qualified", value: "openai/gpt-5-mini" }, - }, + modelChanged: true, trackRunId: "stale-command-run", }); await draining; @@ -5084,6 +5094,7 @@ describe("handleSendChat", () => { const item = createQueuedLocalCommand("reconnected-model-command", "/model gpt-5-mini", { sessionKey: "agent:main:first", }); + const refreshTools = vi.fn(async () => undefined); const host = makeChatHost({ requestHandlers: { "chat.history": () => idleChatHistory(item.sessionKey), @@ -5091,8 +5102,8 @@ describe("handleSendChat", () => { connectionEpoch: 1, chatQueue: [item], sessionKey: item.sessionKey, + refreshCurrentSessionTools: refreshTools, }); - const setModelOverride = vi.spyOn(host.sessions, "setModelOverride"); expect(admitQueuedMessageForSession(host, item.sessionKey, item)).toBe(true); const draining = retryReconnectableQueuedChatSends(host); @@ -5103,13 +5114,11 @@ describe("handleSendChat", () => { command.resolve({ action: "refresh", content: "Model set to `gpt-5-mini`.", - sessionPatch: { - modelOverride: { kind: "qualified", value: "openai/gpt-5-mini" }, - }, + modelChanged: true, }); await draining; - expect(setModelOverride).not.toHaveBeenCalled(); + expect(refreshTools).not.toHaveBeenCalled(); expect(host.sessions.state.modelOverrides[item.sessionKey]).toBeUndefined(); }); @@ -5118,7 +5127,7 @@ describe("handleSendChat", () => { { sessionKey: "agent:work:main", mainKey: "main" }, { sessionKey: "agent:work:home", mainKey: "home" }, ])( - "does not apply a late selected-global model result after the selected agent changes for $sessionKey", + "refreshes model tools only for the still-visible target after an agent selection change ($sessionKey)", async ({ sessionKey, mainKey }) => { const command = createDeferred>>(); executeSlashCommandMock.mockImplementationOnce(() => command.promise); @@ -5126,6 +5135,7 @@ describe("handleSendChat", () => { const item = createQueuedLocalCommand("switched-global-model-command", "/model gpt-5-mini", { sessionKey, }); + const refreshTools = vi.fn(async () => undefined); const host = makeChatHost({ requestHandlers: { "chat.history": () => idleChatHistory(item.sessionKey), @@ -5134,8 +5144,8 @@ describe("handleSendChat", () => { agentsList: { defaultId: "main", mainKey }, chatQueue: [item], sessionKey: item.sessionKey, + refreshCurrentSessionTools: refreshTools, }); - const setModelOverride = vi.spyOn(host.sessions, "setModelOverride"); expect(admitQueuedMessageForSession(host, item.sessionKey, item)).toBe(true); const draining = retryReconnectableQueuedChatSends(host); @@ -5145,13 +5155,11 @@ describe("handleSendChat", () => { command.resolve({ action: "refresh", content: "Model set to `gpt-5-mini`.", - sessionPatch: { - modelOverride: { kind: "qualified", value: "openai/gpt-5-mini" }, - }, + modelChanged: true, }); await draining; - expect(setModelOverride).not.toHaveBeenCalled(); + expect(refreshTools).toHaveBeenCalledTimes(sessionKey === "global" ? 0 : 1); expect(host.sessions.state.modelOverrides[item.sessionKey]).toBeUndefined(); }, ); diff --git a/ui/src/pages/chat/chat-settings-patches.ts b/ui/src/pages/chat/chat-settings-patches.ts index 45d4a4427dfb..3ac3287e88c5 100644 --- a/ui/src/pages/chat/chat-settings-patches.ts +++ b/ui/src/pages/chat/chat-settings-patches.ts @@ -115,7 +115,6 @@ export function patchChatSessionSettings( patch: SessionPatch, options: { agentId?: string; - deferModelOverride?: boolean; ownsModelOverride?: () => boolean; reconcile?: (result: SessionsPatchResult) => Promise | void; } = {}, @@ -127,7 +126,6 @@ export function patchChatSessionSettings( // redirect queued intent to a replacement Gateway. const result = await host.sessions.patch(sessionKey, patch, { agentId: options.agentId, - deferModelOverride: options.deferModelOverride, ownsModelOverride: options.ownsModelOverride, waitFor: previous, }); @@ -169,7 +167,6 @@ export async function patchChatCommandSessionSettings( sessionKey: string, patch: SessionPatch, options: { - deferModelOverride?: boolean; ownsModelOverride?: () => boolean; reconcile?: (result: SessionsPatchResult) => Promise | void; } = {}, diff --git a/ui/src/pages/chat/chat-state.test.ts b/ui/src/pages/chat/chat-state.test.ts index 1e2cde28f2b1..6a0d0de59701 100644 --- a/ui/src/pages/chat/chat-state.test.ts +++ b/ui/src/pages/chat/chat-state.test.ts @@ -1749,7 +1749,7 @@ describe("ChatStateController render lifecycle", () => { toolStreamOrder: [], toolStreamSyncTimer: null, waitingApprovalStatuses: new Map(), - sessions: { setModelOverride: vi.fn() }, + sessions: { refreshReplacement: vi.fn(async () => undefined) }, chatStreamRenderFrame: null, renderLifecycle: { invalidate: requestUpdate }, requestUpdate, @@ -1812,7 +1812,7 @@ describe("ChatStateController render lifecycle", () => { toolStreamById: new Map(), toolStreamOrder: [], toolStreamSyncTimer: null, - sessions: { setModelOverride: vi.fn() } as never, + sessions: { refreshReplacement: vi.fn(async () => undefined) } as never, }); const emitAgent = (seq: number, stream: string, data: Record) => handlePageGatewayEvent(state, { diff --git a/ui/src/pages/chat/chat-view.test.ts b/ui/src/pages/chat/chat-view.test.ts index bf249a931f8d..4b998361073f 100644 --- a/ui/src/pages/chat/chat-view.test.ts +++ b/ui/src/pages/chat/chat-view.test.ts @@ -324,6 +324,7 @@ function createChatHeaderState( overrides: { model?: string | null; modelProvider?: string | null; + modelOverrideSource?: GatewaySessionRow["modelOverrideSource"]; models?: ModelCatalogEntry[]; defaultsThinkingDefault?: string; thinkingDefault?: string; @@ -408,6 +409,7 @@ function createChatHeaderState( return createSessionsListResult({ model: currentModel, modelProvider: currentModelProvider, + modelOverrideSource: overrides.modelOverrideSource, defaultsThinkingDefault: overrides.defaultsThinkingDefault, thinkingDefault: overrides.thinkingDefault, omitSessionFromList, @@ -434,6 +436,7 @@ function createChatHeaderState( const initialSessionsResult = createSessionsListResult({ model: currentModel, modelProvider: currentModelProvider, + modelOverrideSource: overrides.modelOverrideSource, defaultsThinkingDefault: overrides.defaultsThinkingDefault, thinkingDefault: overrides.thinkingDefault, omitSessionFromList, @@ -6425,10 +6428,12 @@ describe("chat model controls", () => { expect(onThinkingSelect).not.toHaveBeenCalled(); }); - it("omits inherited provenance and resets an override from the provenance row", () => { + it("hides the provenance footer for an inherited default and resets a recorded pin", () => { const { state } = createChatHeaderState({ - model: null, - models: createOpenAiModelCatalog(), + model: "gpt-5", + modelProvider: "openai", + modelOverrideSource: null, + models: [{ id: "gpt-5", name: "GPT-5", provider: "openai" }, ...createOpenAiModelCatalog()], }); const onModelSelect = vi.fn(async () => true); const container = renderModelControls(state); @@ -6436,13 +6441,16 @@ describe("chat model controls", () => { expect(container.querySelector(".chat-controls__model-provenance")).toBeNull(); expect(container.querySelector("[data-chat-model-reset]")).toBeNull(); - renderModelControls( - state, - { modelOverrides: { main: "openai/gpt-5.4" }, onModelSelect }, - container, - ); + state.sessionsResult = createSessionsListResult({ + model: "gpt-5.4", + modelProvider: "openai", + modelOverrideSource: "user", + }); + renderModelControls(state, { onModelSelect }, container); - expect(container.querySelector(".chat-controls__model-provenance")).not.toBeNull(); + expect(container.querySelector(".chat-controls__model-provenance")?.textContent).toContain( + "Only for this session", + ); const reset = container.querySelector("[data-chat-model-reset]"); const modelSelect = getChatModelSelect(container); const details = modelSelect.closest("details"); @@ -6451,7 +6459,7 @@ describe("chat model controls", () => { details.open = true; } expect(reset).toBeInstanceOf(HTMLButtonElement); - expect(reset?.textContent?.trim()).toBe("Use default"); + expect(reset?.textContent?.trim()).toBe("Use default (GPT-5)"); reset?.focus(); reset?.click(); expect(onModelSelect).toHaveBeenCalledWith("", "main"); @@ -6460,6 +6468,58 @@ describe("chat model controls", () => { container.remove(); }); + // Settings can move the agent default onto — and back off — a session's pinned + // model. Provenance must survive both moves, and the default row must stay a live + // way to clear the pin while the two values coincide. + it("keeps a session pin selectable and clearable when the agent default becomes the pinned model", () => { + const { state } = createChatHeaderState({ + model: "gpt-5.4", + modelProvider: "openai", + modelOverrideSource: "user", + models: createOpenAiModelCatalog(), + }); + const onModelSelect = vi.fn(async () => true); + // The agent default moves onto the very model this session pinned. + state.sessionsResult = { + ...expectDefined(state.sessionsResult, "sessions result"), + defaults: { + ...expectDefined(state.sessionsResult, "sessions result").defaults, + model: "gpt-5.4", + modelProvider: "openai", + }, + }; + const container = renderModelControls(state, { onModelSelect }); + document.body.append(container); + + expect(container.querySelector(".chat-controls__model-provenance")?.textContent).toContain( + "Only for this session", + ); + const defaultRow = container.querySelector( + '[data-chat-model-option="openai/gpt-5.4"]', + ); + expect(defaultRow?.dataset.chatModelDefault).toBe("true"); + // Pre-fix this row was already the selected "inherited" sentinel, so the click + // was swallowed and the stored pin survived forever. + defaultRow?.click(); + expect(onModelSelect).toHaveBeenCalledWith("", "main"); + + // The default moves away again; the untouched pin is still a pin. + onModelSelect.mockClear(); + state.sessionsResult = { + ...expectDefined(state.sessionsResult, "sessions result"), + defaults: { + ...expectDefined(state.sessionsResult, "sessions result").defaults, + model: "gpt-5", + modelProvider: "openai", + }, + }; + renderModelControls(state, { onModelSelect }, container); + expect(container.querySelector(".chat-controls__model-provenance")?.textContent).toContain( + "Only for this session", + ); + container.remove(); + }); + it("hides model choices for locked sessions while preserving reasoning and speed", () => { const { state } = createReasoningHeaderState({ models: [ @@ -6667,6 +6727,34 @@ describe("chat model controls", () => { expect(visibleOptions[0]?.dataset.chatModelDefault).toBe("true"); }); + it("leaves digit keys to the model search and selects the numbered row from the picker", () => { + const { state } = createChatHeaderState({ + model: "gpt-5.5", + modelProvider: "openai", + models: [ + { id: "gpt-5.5", name: "GPT-5.5", provider: "openai" }, + { id: "claude-sonnet-4-6", name: "Claude Sonnet 4.6", provider: "anthropic" }, + ], + }); + const onModelSelect = vi.fn(async () => true); + const container = renderModelControls(state, { onModelSelect }); + document.body.append(container); + + const details = container.querySelector(".chat-controls__model-picker"); + const search = container.querySelector("[data-chat-model-search]"); + details!.open = true; + search!.value = "claude"; + search!.dispatchEvent(new InputEvent("input", { bubbles: true })); + + search!.dispatchEvent(new KeyboardEvent("keydown", { key: "1", bubbles: true })); + expect(onModelSelect).not.toHaveBeenCalled(); + expect(search!.value).toBe("claude"); + + details!.dispatchEvent(new KeyboardEvent("keydown", { key: "1", bubbles: true })); + expect(onModelSelect).toHaveBeenCalledWith("anthropic/claude-sonnet-4-6", "main"); + container.remove(); + }); + it("groups legacy Codex model references under OpenAI", () => { const { state } = createChatHeaderState({ model: "gpt-5.5", @@ -7425,7 +7513,6 @@ describe("chat model controls", () => { }, ), refresh: async () => {}, - setModelOverride: vi.fn(), patchRowLocal: vi.fn(), }; const host = { @@ -7500,7 +7587,6 @@ describe("chat model controls", () => { reconciliationStarted.resolve(); await releaseReconciliation.promise; }, - setModelOverride: vi.fn(), patchRowLocal: vi.fn(), }; const host = { @@ -7549,7 +7635,6 @@ describe("chat model controls", () => { }, ), refresh: async () => {}, - setModelOverride: vi.fn(), patchRowLocal: vi.fn(), }; const host = { @@ -7603,13 +7688,6 @@ describe("chat model controls", () => { }, ), 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 = { diff --git a/ui/src/pages/chat/components/chat-model-controls.ts b/ui/src/pages/chat/components/chat-model-controls.ts index efc02514dbee..8bf2aa37b45d 100644 --- a/ui/src/pages/chat/components/chat-model-controls.ts +++ b/ui/src/pages/chat/components/chat-model-controls.ts @@ -186,6 +186,7 @@ export function renderChatModelControls(props: ChatModelControlsProps) { currentOverride, defaultModel, defaultLabel, + modelOverrideSource, options: selectOptions, } = resolveChatModelSelectState({ agentDefaultModel: props.agentDefaultModel, @@ -276,7 +277,6 @@ export function renderChatModelControls(props: ChatModelControlsProps) { } return pickerOption; }); - const explicitOverride = props.modelOverrides?.[props.sessionKey]; const currentCatalogEntry = resolveChatModelCatalogEntry(currentOverride, props.modelCatalog); if ( currentOverride && @@ -303,10 +303,8 @@ export function renderChatModelControls(props: ChatModelControlsProps) { ), }); } - const pickerValue = - !explicitOverride && currentOverride.trim().toLowerCase() === defaultModel.trim().toLowerCase() - ? "" - : currentOverride; + // A persisted pin can match a changed default; equality cannot establish inheritance. + const pickerValue = modelOverrideSource === null ? "" : currentOverride; const activeModelOption = pickerValue === "" ? modelOptions.find((option) => option.isDefault) @@ -439,6 +437,7 @@ export function renderChatModelControls(props: ChatModelControlsProps) { modelOptions, targetGroups: props.modelPickerTargetGroups, selectedModelValue: pickerValue, + sessionModelPinned: modelOverrideSource === "user", sessionKey: props.sessionKey, triggerModelLabel: formatPickerModelLabel(committedModelLabel), triggerStatusLabel: catalogTriggerStatus, diff --git a/ui/src/pages/chat/components/chat-model-picker.ts b/ui/src/pages/chat/components/chat-model-picker.ts index fac603318eea..ef458bcb1969 100644 --- a/ui/src/pages/chat/components/chat-model-picker.ts +++ b/ui/src/pages/chat/components/chat-model-picker.ts @@ -39,6 +39,8 @@ type ChatModelPickerParams = { open?: boolean; targetGroups?: readonly ChatModelPickerTargetGroup[]; selectedModelValue: string; + /** Recorded user pin, so the footer never offers a reset for an inherited default. */ + sessionModelPinned: boolean; sessionKey: string; triggerModelLabel: string; triggerStatusLabel?: string; @@ -101,6 +103,9 @@ function highlightModelRow(menu: HTMLElement, row: HTMLButtonElement | undefined } } +// Numbers follow the filtered order because digit selection reads that same row list. +// A focused search input owns the digits instead (handleModelPickerKeydown bails on input +// targets), and the :focus-within rule in styles/chat/layout.css withdraws these keycaps there. function updateModelShortcuts(menu: HTMLElement, rows: readonly HTMLButtonElement[]): void { menu.querySelectorAll("[data-chat-model-shortcut]").forEach((shortcut) => { shortcut.hidden = true; @@ -613,45 +618,40 @@ export function renderChatModelPicker(params: ChatModelPickerParams) { ${params.contextWindow ? renderContextWindowControl(params.contextWindow, params.sessionKey) : nothing} - ${params.modelOptions.length > 0 && params.selectedModelValue !== "" + ${params.sessionModelPinned && params.modelOptions.length > 0 ? html`
- ${t("chat.modelControls.sessionOverride")} - ${t("chat.modelControls.onlyForSession")} + - + ${t("chat.modelControls.useDefaultModel", { + model: params.defaultModelLabel, + })} +
` : nothing} ` diff --git a/ui/src/pages/chat/run-lifecycle.test.ts b/ui/src/pages/chat/run-lifecycle.test.ts index 8bff9567577b..238706c47a19 100644 --- a/ui/src/pages/chat/run-lifecycle.test.ts +++ b/ui/src/pages/chat/run-lifecycle.test.ts @@ -703,7 +703,6 @@ describe("reconcileChatRunFromCurrentSessionRow stale-active suppression (#87875 ]), sessions: { reconcileRunTerminal, - setModelOverride: vi.fn(), }, }); @@ -738,7 +737,6 @@ describe("reconcileChatRunFromCurrentSessionRow stale-active suppression (#87875 sessionsResult: null, sessions: { reconcileRunTerminal, - setModelOverride: vi.fn(), }, }); diff --git a/ui/src/pages/chat/run-lifecycle.ts b/ui/src/pages/chat/run-lifecycle.ts index 381dd6d8dd8a..0d9ef8bd43f8 100644 --- a/ui/src/pages/chat/run-lifecycle.ts +++ b/ui/src/pages/chat/run-lifecycle.ts @@ -69,7 +69,7 @@ type RunLifecycleHost = Omit< chatRunStatus?: ChatRunUiStatus | null; chatRunStatusClearTimer?: TimerHandle | number | null; sessionsResult?: SessionsListResult | null; - sessions?: Partial>; + sessions?: Partial>; lastLocalTerminalReconcile?: LocalTerminalReconcile | null; requestUpdate?: () => void; }; diff --git a/ui/src/pages/chat/tool-stream-fallback.node.test.ts b/ui/src/pages/chat/tool-stream-fallback.node.test.ts index cadcccafcc0d..86a80186cf2d 100644 --- a/ui/src/pages/chat/tool-stream-fallback.node.test.ts +++ b/ui/src/pages/chat/tool-stream-fallback.node.test.ts @@ -1,5 +1,10 @@ // @vitest-environment node import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { createDeferred } from "../../../../test/helpers/promise.js"; +import type { GatewayBrowserClient } from "../../api/gateway.ts"; +import type { SessionsListResult } from "../../api/types.ts"; +import { createSessionCapability } from "../../lib/sessions/index.ts"; +import { waitForFast } from "../../test-helpers/wait-for.ts"; import { agentEvent, createHost, @@ -180,63 +185,127 @@ describe("app-tool-stream fallback lifecycle handling", () => { vi.useRealTimers(); }); - it("updates the chat model cache from session_status model changes", () => { - const host = createHost(); - - handleAgentEvent(host, { - runId: "run-1", - seq: 1, - stream: "tool", - ts: Date.now(), - sessionKey: "main", - data: { - phase: "result", - name: "session_status", - toolCallId: "status-1", - result: { - details: { - ok: true, - sessionKey: "main", - changedModel: true, - modelProvider: "anthropic", - model: "claude-sonnet-4-6", - modelOverride: "anthropic/claude-sonnet-4-6", - }, + it.each([ + ["main", "agent:main:main", "main", null], + ["agent:work:thread", "agent:work:thread", "work", "openai/gpt-5-mini"], + ["global", "global", "work", null], + ["agent:work:main", "global", "work", null], + ])( + "refreshes canonical selection after status changes for %s", + async (key, target, agentId, override) => { + const row = { + key, + kind: "direct" as const, + updatedAt: 1, + modelProvider: "openai", + model: "gpt-5.6-sol", + modelOverrideSource: null, + }; + const result: SessionsListResult = { + ts: 1, + path: "(multiple)", + count: 1, + defaults: { modelProvider: null, model: null, contextTokens: null }, + sessions: [row], + }; + const pendingPatch = createDeferred(); + const request = vi.fn(async (method: string) => + method === "sessions.patch" ? pendingPatch.promise : result, + ); + const sessions = createSessionCapability({ + snapshot: { + client: { request } as unknown as GatewayBrowserClient, + phase: "connected", + hello: null, + assistantAgentId: agentId, + sessionKey: key, }, - }, - }); - - expect(host.sessions.state.modelOverrides.main).toBe("anthropic/claude-sonnet-4-6"); - }); - - it("clears the chat model cache from session_status default resets", () => { - const host = createHost(); - host.sessions.setModelOverride("main", "anthropic/claude-sonnet-4-6"); - - handleAgentEvent(host, { - runId: "run-1", - seq: 1, - stream: "tool", - ts: Date.now(), - sessionKey: "main", - data: { - phase: "result", - name: "session_status", - toolCallId: "status-1", - result: { - details: { - ok: true, - sessionKey: "main", - changedModel: true, - modelProvider: "openai", - model: "gpt-5.4", - modelOverride: null, + subscribe: () => () => undefined, + subscribeEvents: () => () => undefined, + }); + const host = createHost({ + sessionKey: key, + assistantAgentId: agentId, + agentsList: { defaultId: "main" }, + sessions, + }); + const event = { + ...agentEvent( + "run-1", + 1, + "tool", + { + phase: "result", + name: "session_status", + toolCallId: "status-1", + result: { + details: { changedModel: true, sessionKey: target, agentId, modelOverride: override }, + }, }, - }, - }, - }); + key, + ), + agentId, + }; + handleAgentEvent(host, event); + await waitForFast(() => + expect(sessions.state.result?.sessions[0]?.model).toBe("gpt-5.6-sol"), + ); + expect(request).toHaveBeenCalledWith("sessions.list", expect.objectContaining({ agentId })); + expect(sessions.state.modelOverrides).toEqual({}); - expect(host.sessions.state.modelOverrides.main).toBeNull(); + // Replaying an old tool result reads today's row, without replacing a newer UI intent. + result.sessions = [{ ...row, model: "gpt-5-mini" }]; + const patch = sessions.patch(key, { model: "openai/gpt-5.6-luna" }); + handleAgentEvent( + createHost({ + sessionKey: key, + assistantAgentId: agentId, + agentsList: { defaultId: "main" }, + sessions, + }), + event, + ); + await waitForFast(() => + expect(request.mock.calls.filter(([method]) => method === "sessions.list")).toHaveLength(2), + ); + await waitForFast(() => expect(sessions.state.loading).toBe(false)); + expect(sessions.state.result?.sessions[0]?.model).toBe("gpt-5-mini"); + expect(sessions.state.modelOverrides[key]).toBe("openai/gpt-5.6-luna"); + pendingPatch.resolve({ ok: true, key, entry: {} }); + await patch; + expect(sessions.state.modelOverrides).toEqual({}); + sessions.dispose(); + }, + ); + + it.each([ + { changedModel: true, sessionKey: "global" }, + { changedModel: false, sessionKey: "global", agentId: "work" }, + { changedModel: true, sessionKey: "agent:work:other", agentId: "work" }, + { changedModel: true, sessionKey: "global", agentId: "main" }, + ])("does not refresh an unrelated/read-only status result (%j)", (details) => { + const host = createHost({ + sessionKey: "global", + assistantAgentId: "work", + agentsList: { defaultId: "main" }, + }); + handleAgentEvent(host, { + ...agentEvent( + "run-1", + 1, + "tool", + { + phase: "result", + name: "session_status", + toolCallId: "status-1", + result: { details }, + }, + "global", + ), + agentId: "work", + }); + expect(host.sessions.refreshReplacement).not.toHaveBeenCalled(); + expect(host.sessions.state.modelOverrides).toEqual({}); }); it("tags stream segments with the tool they precede without resetting elapsed time", () => { diff --git a/ui/src/pages/chat/tool-stream.node.test.ts b/ui/src/pages/chat/tool-stream.node.test.ts index b016f3745f0b..4226ab27037f 100644 --- a/ui/src/pages/chat/tool-stream.node.test.ts +++ b/ui/src/pages/chat/tool-stream.node.test.ts @@ -579,13 +579,15 @@ describe("app-tool-stream result blocks", () => { details: { changedModel: true, sessionKey: "main", + agentId: "main", modelOverride: "openai/gpt-5.6-luna", }, }, }), ); - expect(host.sessions.state.modelOverrides.main).toBe("openai/gpt-5.6-luna"); + expect(host.sessions.refreshReplacement).toHaveBeenCalledOnce(); + expect(host.sessions.state.modelOverrides).toEqual({}); }, ); diff --git a/ui/src/pages/chat/tool-stream.test-helpers.ts b/ui/src/pages/chat/tool-stream.test-helpers.ts index d52d38cc22bf..993b825a05de 100644 --- a/ui/src/pages/chat/tool-stream.test-helpers.ts +++ b/ui/src/pages/chat/tool-stream.test-helpers.ts @@ -6,7 +6,7 @@ type AgentEvent = NonNullable[1]>; type MutableHost = ToolStreamHost & { sessions: { state: { modelOverrides: Record }; - setModelOverride: (key: string, value: string | null | undefined) => void; + refreshReplacement: (agentId?: string | null) => Promise; }; compactionStatus?: unknown; compactionClearTimer?: number | null; @@ -33,13 +33,7 @@ export function createHost(overrides?: Partial): MutableHost { toolStreamSyncTimer: null, sessions: { state: { modelOverrides }, - setModelOverride: (key, value) => { - if (value === undefined) { - delete modelOverrides[key]; - } else { - modelOverrides[key] = value; - } - }, + refreshReplacement: vi.fn(async () => undefined), }, compactionStatus: null, compactionClearTimer: null, diff --git a/ui/src/pages/chat/tool-stream.ts b/ui/src/pages/chat/tool-stream.ts index 0f7b4f51927f..f475943d82ca 100644 --- a/ui/src/pages/chat/tool-stream.ts +++ b/ui/src/pages/chat/tool-stream.ts @@ -95,7 +95,7 @@ export type ToolStreamHost = { waitingApprovalStatuses?: Map; waitingApprovalResolvedIds?: Set; requestUpdate?: () => void; - sessions: Pick; + sessions: Pick; }; function resolveModelLabel(provider: unknown, model: unknown): string | null { @@ -220,35 +220,18 @@ function readLiveDiffStat(value: unknown): DiffStat | undefined { : undefined; } -function resolveSessionStatusModelOverride( - details: Record | null, -): string | null | undefined { - if (details?.changedModel !== true) { - return undefined; - } - if (Object.hasOwn(details, "modelOverride")) { - const override = toTrimmedString(details.modelOverride); - return override; - } - const model = toTrimmedString(details.model); - if (!model) { - return undefined; - } - const provider = toTrimmedString(details.modelProvider); - return provider ? `${provider}/${model}` : model; -} - -function syncSessionStatusModelOverride(host: ToolStreamHost, data: Record) { +function refreshSessionStatusModel(host: ToolStreamHost, data: Record) { const details = readRecord(readRecord(data.result)?.details); - const targetSessionKey = toTrimmedString(details?.sessionKey) ?? host.sessionKey; - if (!uiSessionEventMatches(host, targetSessionKey, toTrimmedString(details?.agentId))) { + if (details?.changedModel !== true) { return; } - const override = resolveSessionStatusModelOverride(details); - if (override === undefined) { + const targetSessionKey = toTrimmedString(details.sessionKey) ?? host.sessionKey; + const agentId = toTrimmedString(details.agentId); + if (!agentId || !uiSessionEventMatches(host, targetSessionKey, agentId)) { return; } - host.sessions.setModelOverride(targetSessionKey, override); + // Results can be replayed from history; read current truth without replacing pending UI intent. + void host.sessions.refreshReplacement(agentId); } function buildToolStreamMessage(entry: ToolStreamEntry): Record { @@ -1150,7 +1133,7 @@ export function handleAgentEvent(host: ToolStreamHost, payload?: AgentEventPaylo : undefined; const liveDiffStat = phase === "input_delta" ? readLiveDiffStat(data.diff) : undefined; if (name === "session_status" && phase === "result") { - syncSessionStatusModelOverride(host, data); + refreshSessionStatusModel(host, data); } const now = Date.now(); diff --git a/ui/src/test-helpers/chat-model.ts b/ui/src/test-helpers/chat-model.ts index 08b0f2f16700..32e0d73d8b56 100644 --- a/ui/src/test-helpers/chat-model.ts +++ b/ui/src/test-helpers/chat-model.ts @@ -57,6 +57,7 @@ export function createSessionsListResult( params: { model?: string | null; modelProvider?: string | null; + modelOverrideSource?: GatewaySessionRow["modelOverrideSource"]; defaultsModel?: string | null; defaultsProvider?: string | null; defaultsThinkingLevels?: SessionsListResult["defaults"]["thinkingLevels"]; @@ -69,6 +70,7 @@ export function createSessionsListResult( const { model = null, modelProvider = model ? "openai" : null, + modelOverrideSource, defaultsModel = "gpt-5", defaultsProvider = defaultsModel ? "openai" : null, defaultsThinkingLevels, @@ -96,6 +98,7 @@ export function createSessionsListResult( createMainSessionRow({ ...(modelProvider ? { modelProvider } : {}), ...(model ? { model } : {}), + ...(modelOverrideSource === undefined ? {} : { modelOverrideSource }), ...(thinkingDefault ? { thinkingDefault } : {}), }), ], diff --git a/ui/src/test-helpers/control-ui-e2e.ts b/ui/src/test-helpers/control-ui-e2e.ts index c0821616c8e6..2615abd45363 100644 --- a/ui/src/test-helpers/control-ui-e2e.ts +++ b/ui/src/test-helpers/control-ui-e2e.ts @@ -1403,6 +1403,9 @@ function installControlUiMockGateway( patch.lastReadAt = sessionPatchTimestamp; patch.markedUnreadAt = undefined; } + if (hasOwn(params, "model")) { + patch.modelOverrideSource = params.model == null ? null : "user"; + } if (scenario.sessionArchiveFiltering && hasOwn(params, "archived")) { patch.archived = params.archived; } @@ -1578,6 +1581,7 @@ function installControlUiMockGateway( label: "Main", model: "gpt-5.5", modelProvider: "openai", + modelOverrideSource: null, status: "done", totalTokens: 0, updatedAt: Date.now(),