From f010c00aceac8b625b128e9e1d6d41bbda02bd9d Mon Sep 17 00:00:00 2001 From: "Vyctor H. Brzezowski" Date: Wed, 26 Aug 2026 19:02:54 -0300 Subject: [PATCH] fix(ui): distinguish inherited model defaults from session pins (#120805) * fix(ui): distinguish inherited model defaults from session pins The Gateway session projection collapsed inherited agent defaults and persisted conversation pins into the same effective model fields, so the Control UI picker marked a row Default while its footer simultaneously called it a session override and offered a meaningless reset. Project the authoritative persisted provenance instead: the session owner resolves `modelOverrideSource` (normalizing entries written before source tracking), `buildGatewaySessionRow` and the session change event carry the additive `"user" | "auto" | null` protocol field, and the Control UI treats only `"user"` as a conversation pin. Closes #120752 * test(ui): scope model provenance to active pane * fix(ui): derive the model picker's inherited sentinel from recorded provenance Deriving "inherits the agent default" from the effective model matching the default meant a session pinned to model X stopped reading as pinned once the agent default moved to X. The picker then rendered the default row as already selected, so clicking it matched its own commit value and was swallowed, leaving the stored pin unreachable from the UI. Project the session store's existing modelOverrideSource onto the gateway row and let the UI resolve a closed selection source (inherited/pinned/fallback) from it, falling back to the old equality guess only for gateways too old to send the marker. Preserve the null tombstone in reconcile so a cleared pin stays distinguishable from that silence. * test(ui): pin the model picker's digit-key ownership The numbered keycaps and the digit handler are paired by search focus, but the guard that keeps digits in the search input had no coverage outside the browser e2e lane. Add a unit regression for both halves and record the contract at the numbering site, where reviewers keep reading the two gates as unrelated. * fix(ui): keep session model selection provenance authoritative Project saved model provenance through Gateway rows and events, retire temporary UI claims at their owner, and use canonical refresh after slash and status model changes. Preserve model intent through rollback and deletion without retaining a public cache setter. Co-authored-by: Vyctor H. Brzezowski * refactor(ui): keep recovery with scoped session operations Preserve recovery notifications, visible errors, and connection checks while keeping the optimistic mutation owner within its module limit. Co-authored-by: Vyctor H. Brzezowski --------- Co-authored-by: Peter Steinberger --- .../OpenClawProtocol/GatewayModels.swift | 4 + .../src/schema/sessions-row.test.ts | 20 ++ .../src/schema/sessions-row.ts | 4 + .../openclaw-tools.session-status.test.ts | 69 +++-- src/agents/tools/session-status-tool.ts | 2 + .../model-override-provenance.test.ts | 38 ++- .../sessions/model-override-provenance.ts | 22 ++ .../sessions/reset-preserved-selection.ts | 13 +- src/gateway/session-event-payload.test.ts | 36 +++ src/gateway/session-event-payload.ts | 2 + src/gateway/session-utils-owners.test.ts | 2 +- src/gateway/session-utils-row.ts | 2 + src/gateway/session-utils.test.ts | 44 +++ src/gateway/session-utils.types.ts | 1 + ...chat-flow.model-picker-refresh.e2e.test.ts | 57 ++++ ui/src/i18n/locales/en.ts | 5 +- ui/src/lib/chat/model-select-state.test.ts | 75 +++++ ui/src/lib/chat/model-select-state.ts | 11 + .../lib/sessions/index-subscriptions.test.ts | 2 + .../sessions/index.create-background.test.ts | 157 ++++++---- ui/src/lib/sessions/index.recovery.test.ts | 59 ++++ ui/src/lib/sessions/index.ts | 18 +- ui/src/lib/sessions/list-options.test.ts | 40 ++- ui/src/lib/sessions/patch.ts | 2 - ui/src/lib/sessions/reconcile.test.ts | 6 + ui/src/lib/sessions/reconcile.ts | 13 +- ui/src/lib/sessions/session-capability.ts | 1 - .../sessions/session-model-override.test.ts | 48 ++- ui/src/lib/sessions/session-mutations.ts | 90 +++--- .../lib/sessions/session-scoped-operations.ts | 25 ++ .../pages/chat/chat-command-executor.test.ts | 281 ++++-------------- ui/src/pages/chat/chat-command-executor.ts | 59 +--- ui/src/pages/chat/chat-commands.ts | 2 +- .../pages/chat/chat-history.inflight.test.ts | 2 +- ui/src/pages/chat/chat-history.test.ts | 34 ++- ui/src/pages/chat/chat-send.test.ts | 74 +++-- ui/src/pages/chat/chat-settings-patches.ts | 3 - ui/src/pages/chat/chat-state.test.ts | 4 +- ui/src/pages/chat/chat-view.test.ts | 118 ++++++-- .../chat/components/chat-model-controls.ts | 9 +- .../chat/components/chat-model-picker.ts | 74 ++--- ui/src/pages/chat/run-lifecycle.test.ts | 2 - ui/src/pages/chat/run-lifecycle.ts | 2 +- .../chat/tool-stream-fallback.node.test.ts | 177 +++++++---- ui/src/pages/chat/tool-stream.node.test.ts | 4 +- ui/src/pages/chat/tool-stream.test-helpers.ts | 10 +- ui/src/pages/chat/tool-stream.ts | 35 +-- ui/src/test-helpers/chat-model.ts | 3 + ui/src/test-helpers/control-ui-e2e.ts | 4 + 49 files changed, 1105 insertions(+), 660 deletions(-) create mode 100644 ui/src/lib/sessions/index.recovery.test.ts 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(),