diff --git a/src/browser/App.tsx b/src/browser/App.tsx index 6fb376dd4c..5b7591d3bd 100644 --- a/src/browser/App.tsx +++ b/src/browser/App.tsx @@ -85,11 +85,7 @@ import { getRuntimeTypeForTelemetry } from "@/common/telemetry"; import { useStartWorkspaceCreation } from "./hooks/useStartWorkspaceCreation"; import { useAPI } from "@/browser/contexts/API"; import { requestActiveTurnThinkingLevel } from "@/browser/utils/activeTurnThinking"; -import { - clearPendingWorkspaceAiSettings, - markPendingWorkspaceAiSettings, - resolveEffectiveComposerModel, -} from "@/browser/utils/workspaceAiSettingsSync"; +import { resolveEffectiveComposerModel } from "@/browser/utils/workspaceAiSettingsSync"; import { AuthTokenModal } from "@/browser/components/AuthTokenModal/AuthTokenModal"; import { ScratchPage } from "@/browser/components/ScratchPage/ScratchPage"; @@ -540,8 +536,6 @@ function AppInner() { const normalized = THINKING_LEVELS.includes(level) ? level : "off"; const model = getModelForWorkspace(workspaceId); const key = getThinkingLevelKey(workspaceId); - // Carry the current pro-mode choice: the backend replaces the agent's - // settings wholesale, so omitting reasoningMode would wipe it. const reasoningMode = getReasoningModeForWorkspace(workspaceId); // Use the utility function which handles localStorage and event dispatch @@ -573,30 +567,7 @@ function AppInner() { {} ); - // Persist to backend so the palette change follows the workspace across devices. if (api) { - markPendingWorkspaceAiSettings(workspaceId, normalizedAgentId, { - model, - thinkingLevel: normalized, - reasoningMode, - }); - - api.workspace - .updateAgentAISettings({ - workspaceId, - agentId: normalizedAgentId, - aiSettings: { model, thinkingLevel: normalized, reasoningMode }, - }) - .then((result) => { - if (!result.success) { - clearPendingWorkspaceAiSettings(workspaceId, normalizedAgentId); - } - }) - .catch(() => { - clearPendingWorkspaceAiSettings(workspaceId, normalizedAgentId); - // Best-effort only. - }); - // Mid-turn change: also apply to the active turn's next model step so // the palette/keybind path behaves like the selector (ThinkingProvider). requestActiveTurnThinkingLevel(api, workspaceId, normalized); @@ -614,9 +585,7 @@ function AppInner() { [api, getModelForWorkspace, getReasoningModeForWorkspace] ); - // Palette toggle for the OpenAI pro reasoning mode. Persists like the - // thinking-level palette action: localStorage first (ThinkingProvider listens), - // then best-effort backend sync with the full settings payload. + // Keep palette choices local until a user message sends the full settings. const toggleReasoningModeFromPalette = useCallback( (workspaceId: string) => { if (!workspaceId) { @@ -654,32 +623,8 @@ function AppInner() { }, {} ); - - if (api) { - markPendingWorkspaceAiSettings(workspaceId, normalizedAgentId, { - model, - thinkingLevel, - reasoningMode: next, - }); - - api.workspace - .updateAgentAISettings({ - workspaceId, - agentId: normalizedAgentId, - aiSettings: { model, thinkingLevel, reasoningMode: next }, - }) - .then((result) => { - if (!result.success) { - clearPendingWorkspaceAiSettings(workspaceId, normalizedAgentId); - } - }) - .catch(() => { - clearPendingWorkspaceAiSettings(workspaceId, normalizedAgentId); - // Best-effort only. - }); - } }, - [api, getModelForWorkspace, getReasoningModeForWorkspace, getThinkingLevelForWorkspace] + [getModelForWorkspace, getReasoningModeForWorkspace, getThinkingLevelForWorkspace] ); const getFastModeActive = useCallback(() => { diff --git a/src/browser/components/WorkspaceModeAISync/WorkspaceModeAISync.test.tsx b/src/browser/components/WorkspaceModeAISync/WorkspaceModeAISync.test.tsx index aab70da540..0c00cdf707 100644 --- a/src/browser/components/WorkspaceModeAISync/WorkspaceModeAISync.test.tsx +++ b/src/browser/components/WorkspaceModeAISync/WorkspaceModeAISync.test.tsx @@ -95,7 +95,7 @@ describe("WorkspaceModeAISync", () => { expect(consumeWorkspaceModelChange(workspaceId, planModel)).toBe("agent"); }); - test("prefers configured agent defaults over workspace-by-agent overrides", async () => { + test("preserves unsent workspace choices over configured agent defaults", async () => { const workspaceId = nextWorkspaceId(); const configuredModel = "anthropic:claude-haiku-4-5"; @@ -116,8 +116,8 @@ describe("WorkspaceModeAISync", () => { renderSync({ workspaceId, agentId: "exec" }); await waitFor(() => { - expect(readPersistedState(getModelKey(workspaceId), "")).toBe(configuredModel); - expect(readPersistedState(getThinkingLevelKey(workspaceId), "high")).toBe(configuredThinking); + expect(readPersistedState(getModelKey(workspaceId), "")).toBe("some-legacy-model"); + expect(readPersistedState(getThinkingLevelKey(workspaceId), "high")).toBe("medium"); }); }); diff --git a/src/browser/contexts/ThinkingContext.test.tsx b/src/browser/contexts/ThinkingContext.test.tsx index 09e899bc44..b71189df78 100644 --- a/src/browser/contexts/ThinkingContext.test.tsx +++ b/src/browser/contexts/ThinkingContext.test.tsx @@ -323,6 +323,7 @@ describe("ThinkingContext", () => { test("setting thinking uses metadata model before global default", async () => { const workspaceId = "ws-set-thinking-metadata-model"; + updatePersistedState(getReasoningModeKey(workspaceId), "pro"); const updateAgentAISettings = mock< (args: WorkspaceUpdateAgentAISettingsArgs) => Promise >(() => @@ -357,24 +358,16 @@ describe("ThinkingContext", () => { button.click(); }); - // setThinkingLevel persists the full settings payload including the current - // reasoningMode (default "standard") so partial writes cannot clobber it. const expectedSettings = { model: "metadataModel:abc", thinkingLevel: "medium" as const, - reasoningMode: "standard" as const, + reasoningMode: "pro" as const, }; await waitFor(() => { expect(readWorkspaceAISettingsCache(workspaceId).exec).toEqual(expectedSettings); }, METADATA_WAIT_OPTIONS); - if (updateAgentAISettings.mock.calls.length > 0) { - expect(updateAgentAISettings).toHaveBeenCalledWith({ - workspaceId, - agentId: "exec", - aiSettings: expectedSettings, - }); - } + expect(updateAgentAISettings).not.toHaveBeenCalled(); }); test("setting thinking preserves an explicit Coder gateway model identity", async () => { @@ -427,6 +420,7 @@ describe("ThinkingContext", () => { await waitFor(() => { expect(readWorkspaceAISettingsCache(workspaceId).exec).toEqual(expectedSettings); }, METADATA_WAIT_OPTIONS); + expect(updateAgentAISettings).not.toHaveBeenCalled(); }); test("self-heals corrupt persisted reasoningMode to standard but keeps valid pro", async () => { @@ -634,13 +628,7 @@ describe("ThinkingContext", () => { expect(readWorkspaceAISettingsCache(workspaceId).exec).toEqual(expectedSettings); }, METADATA_WAIT_OPTIONS); - if (updateAgentAISettings.mock.calls.length > 0) { - expect(updateAgentAISettings).toHaveBeenCalledWith({ - workspaceId, - agentId: "exec", - aiSettings: expectedSettings, - }); - } + expect(updateAgentAISettings).not.toHaveBeenCalled(); }); test("requests a mid-turn override for the active workspace turn on slider changes", async () => { diff --git a/src/browser/contexts/ThinkingContext.tsx b/src/browser/contexts/ThinkingContext.tsx index 0820b9f333..946a69141b 100644 --- a/src/browser/contexts/ThinkingContext.tsx +++ b/src/browser/contexts/ThinkingContext.tsx @@ -28,11 +28,7 @@ import { useMinThinkingLevels } from "@/browser/hooks/useMinThinkingLevels"; import { useProvidersConfig } from "@/browser/hooks/useProvidersConfig"; import { useAPI } from "@/browser/contexts/API"; import { requestActiveTurnThinkingLevel } from "@/browser/utils/activeTurnThinking"; -import { - clearPendingWorkspaceAiSettings, - getWorkspaceAiSettingsFromMetadata, - markPendingWorkspaceAiSettings, -} from "@/browser/utils/workspaceAiSettingsSync"; +import { getWorkspaceAiSettingsFromMetadata } from "@/browser/utils/workspaceAiSettingsSync"; import { useOptionalWorkspaceContext } from "@/browser/contexts/WorkspaceContext"; import { KEYBINDS, matchesKeybind } from "@/browser/utils/ui/keybinds"; import { WORKSPACE_DEFAULTS } from "@/constants/workspaceDefaults"; @@ -133,17 +129,13 @@ export const ThinkingProvider: React.FC = (props) => { updatePersistedState(thinkingKey, legacy); }, [defaultModel, scopeId, thinkingKey]); - // Shared persistence for both setters: caches the full per-agent settings and - // pushes them to the backend. updateAgentAISettings replaces the agent's - // settings wholesale, so every payload must carry BOTH thinkingLevel and - // reasoningMode or the omitted one gets wiped on the next sync. + // Keep picker choices local until a user message sends the full settings. const persistAgentAiSettings = useCallback( (settings: { model: string; thinkingLevel: ThinkingLevel; reasoningMode: OpenAIReasoningMode; }) => { - // Workspace variant: persist to backend so settings follow the workspace across devices. if (!props.workspaceId) { return; } @@ -174,38 +166,12 @@ export const ThinkingProvider: React.FC = (props) => { }, {} ); - - if (!api) { - return; - } - - // Avoid stale backend metadata clobbering newer local preferences when users - // click through levels quickly (tests reproduce this by cycling to xhigh). - markPendingWorkspaceAiSettings(workspaceId, normalizedAgentId, settings); - - api.workspace - .updateAgentAISettings({ - workspaceId, - agentId: normalizedAgentId, - aiSettings: settings, - }) - .then((result) => { - if (!result.success) { - clearPendingWorkspaceAiSettings(workspaceId, normalizedAgentId); - } - }) - .catch(() => { - clearPendingWorkspaceAiSettings(workspaceId, normalizedAgentId); - // Best-effort only. If offline or backend is old, the next sendMessage will persist. - }); }, - [api, props.workspaceId, scopeId] + [props.workspaceId, scopeId] ); // Read the sibling setting at call time (not from the render closure) so // rapid interleaved updates cannot persist a stale counterpart value. - // Coerced like the render path: a corrupt persisted value must not ride a - // thinking-level change into updateAgentAISettings and fail backend sync. const getCurrentReasoningMode = useCallback( (): OpenAIReasoningMode => coerceOpenAIReasoningMode( diff --git a/src/browser/contexts/WorkspaceContext.test.tsx b/src/browser/contexts/WorkspaceContext.test.tsx index 433f7693d2..ec8bdaf0bd 100644 --- a/src/browser/contexts/WorkspaceContext.test.tsx +++ b/src/browser/contexts/WorkspaceContext.test.tsx @@ -20,7 +20,7 @@ import { import { SCRATCH_PROJECT_CONFIG_KEY } from "@/common/constants/scratch"; import { MULTI_PROJECT_CONFIG_KEY } from "@/common/constants/multiProject"; import type { RecursivePartial } from "@/browser/testUtils"; -import { readPersistedState } from "@/browser/hooks/usePersistedState"; +import { readPersistedState, updatePersistedState } from "@/browser/hooks/usePersistedState"; import { getProjectRouteId } from "@/common/utils/projectRouteId"; import type { RightSidebarLayoutState } from "@/browser/utils/rightSidebarLayout"; @@ -520,15 +520,22 @@ describe("WorkspaceContext", () => { "xhigh" ); }); - test("stale metadata does not override a main workspace agent selection", async () => { + test.each(["unchanged", "mode", "model"])("keeps local choices: %s", async (change) => { + const changed = change !== "unchanged"; + const nextAgentId = change === "mode" ? "auto" : "plan"; const workspaceId = "ws-agent-main"; + const saved = createWorkspaceMetadata({ + id: workspaceId, + agentId: "plan", + aiSettingsByAgent: { plan: { model: "openai:gpt-5.2", thinkingLevel: "high" } }, + }); let emitMetadata: | ((event: { workspaceId: string; metadata: FrontendWorkspaceMetadata | null }) => void) | null = null; createMockAPI({ workspace: { - list: () => Promise.resolve([createWorkspaceMetadata({ id: workspaceId })]), + list: () => Promise.resolve([saved]), onMetadata: () => Promise.resolve( (async function* () { @@ -551,19 +558,34 @@ describe("WorkspaceContext", () => { await waitFor(() => expect(ctx().workspaceMetadata.size).toBe(1)); await waitFor(() => expect(emitMetadata).toBeTruthy()); - expect(ctx().workspaceMetadata.get(workspaceId)?.agentId).toBeUndefined(); + expect(readPersistedState(getAgentIdKey(workspaceId), "")).toBe("plan"); + expect(readPersistedState(getModelKey(workspaceId), "")).toBe("openai:gpt-5.2"); act(() => { + updatePersistedState(getAgentIdKey(workspaceId), "exec"); + updatePersistedState(getModelKey(workspaceId), "anthropic:claude-opus-4-6"); emitMetadata?.({ workspaceId, - metadata: createWorkspaceMetadata({ id: workspaceId, agentId: "plan" }), + metadata: { + ...saved, + title: "Updated title", + ...(changed + ? { + agentId: nextAgentId, + aiSettingsByAgent: { + [nextAgentId]: { model: "openai:gpt-5.3-codex", thinkingLevel: "medium" }, + }, + } + : {}), + }, }); }); - await waitFor(() => expect(ctx().workspaceMetadata.get(workspaceId)?.agentId).toBe("plan")); - expect(readPersistedState(getAgentIdKey(workspaceId), undefined)).toBe( - "exec" + await waitFor(() => + expect(ctx().workspaceMetadata.get(workspaceId)?.title).toBe("Updated title") ); + expect(readPersistedState(getAgentIdKey(workspaceId), "")).toBe("exec"); + expect(readPersistedState(getModelKey(workspaceId), "")).toBe("anthropic:claude-opus-4-6"); }); test("child workspace metadata still seeds the locked backend agent", async () => { diff --git a/src/browser/contexts/WorkspaceContext.tsx b/src/browser/contexts/WorkspaceContext.tsx index 8b1079ceb7..2289a84645 100644 --- a/src/browser/contexts/WorkspaceContext.tsx +++ b/src/browser/contexts/WorkspaceContext.tsx @@ -65,7 +65,6 @@ import { import { normalizeAgentAiDefaults } from "@/common/types/agentAiDefaults"; import { isWorkspaceArchived } from "@/common/utils/archive"; import { appendPinnedTimestamp, reassignPinnedTimestamps } from "@/common/utils/pin"; -import { shouldApplyWorkspaceAiSettingsFromBackend } from "@/browser/utils/workspaceAiSettingsSync"; import { isAbortError } from "@/browser/utils/isAbortError"; import { findAdjacentWorkspaceId } from "@/browser/utils/ui/workspaceDomNav"; import { useRouter } from "@/browser/contexts/RouterContext"; @@ -161,18 +160,19 @@ function migrateLocalGatewayPrefsToBackend( } } -function shouldSeedWorkspaceAgentIdFromBackend(metadata: FrontendWorkspaceMetadata): boolean { - // Main workspaces own their live agent selection in localStorage. Child/task - // workspaces are backend-defined and locked, so they must re-seed from metadata. - return metadata.parentWorkspaceId != null; -} - /** * Seed per-workspace localStorage from backend workspace metadata. * * This keeps a workspace's model/thinking consistent across devices/browsers. */ -function seedWorkspaceLocalStorageFromBackend(metadata: FrontendWorkspaceMetadata): void { +function seedWorkspaceLocalStorageFromBackend( + metadata: FrontendWorkspaceMetadata, + previous?: FrontendWorkspaceMetadata +): void { + // Restore on initial load only; later send echoes must not overwrite unsent choices. + if (metadata.parentWorkspaceId == null && previous != null) { + return; + } // Cache keyed by agentId (string) - includes exec, plan, and custom agents type WorkspaceAISettingsByAgentCache = Partial< Record< @@ -184,7 +184,7 @@ function seedWorkspaceLocalStorageFromBackend(metadata: FrontendWorkspaceMetadat const workspaceId = metadata.id; const metadataAgentId = resolvePersistedAgentId(metadata, ""); - if (shouldSeedWorkspaceAgentIdFromBackend(metadata) && metadataAgentId.length > 0) { + if (metadataAgentId.length > 0) { const key = getAgentIdKey(workspaceId); const normalized = normalizeAgentId(metadataAgentId); const existing = readPersistedState(key, undefined); @@ -215,17 +215,6 @@ function seedWorkspaceLocalStorageFromBackend(metadata: FrontendWorkspaceMetadat if (!entry) continue; if (typeof entry.model !== "string" || entry.model.length === 0) continue; - // Protect newer local preferences from stale metadata updates (e.g., rapid thinking toggles). - if ( - !shouldApplyWorkspaceAiSettingsFromBackend(workspaceId, agentKey, { - model: entry.model, - thinkingLevel: entry.thinkingLevel, - reasoningMode: entry.reasoningMode, - }) - ) { - continue; - } - nextByAgent[agentKey] = { model: entry.model, thinkingLevel: entry.thinkingLevel, @@ -261,9 +250,7 @@ function seedWorkspaceLocalStorageFromBackend(metadata: FrontendWorkspaceMetadat // Absent reasoningMode means "standard": seed it explicitly so switching to // an agent whose settings never carried the field cannot inherit another - // agent's "pro" from the shared workspace-scoped key. Newer local choices - // are already protected by the pending-settings guard above - // (shouldApplyWorkspaceAiSettingsFromBackend). + // agent's "pro" from the shared workspace-scoped key. const reasoningKey = getReasoningModeKey(workspaceId); const nextReasoning = active.reasoningMode ?? "standard"; const existingReasoning = readPersistedState( @@ -1090,7 +1077,10 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { ensureCreatedAt(metadata); // Use stable workspace ID as key (not path, which can change) - seedWorkspaceLocalStorageFromBackend(metadata); + seedWorkspaceLocalStorageFromBackend( + metadata, + workspaceMetadataRef.current.get(metadata.id) + ); metadataMap.set(metadata.id, metadata); } @@ -1259,7 +1249,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { // 1. ALWAYS normalize incoming metadata first - this is the critical data update. if (meta !== null) { ensureCreatedAt(meta); - seedWorkspaceLocalStorageFromBackend(meta); + seedWorkspaceLocalStorageFromBackend(meta, workspaceMetadataRef.current.get(meta.id)); } const isNowArchived = @@ -1417,7 +1407,10 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { // Update metadata immediately to avoid race condition with validation effect ensureCreatedAt(result.metadata); - seedWorkspaceLocalStorageFromBackend(result.metadata); + seedWorkspaceLocalStorageFromBackend( + result.metadata, + workspaceMetadataRef.current.get(result.metadata.id) + ); setWorkspaceMetadata((prev) => { const updated = new Map(prev); updated.set(result.metadata.id, result.metadata); @@ -1805,7 +1798,10 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { const metadata = await api.workspace.getInfo({ workspaceId }); if (metadata) { ensureCreatedAt(metadata); - seedWorkspaceLocalStorageFromBackend(metadata); + seedWorkspaceLocalStorageFromBackend( + metadata, + workspaceMetadataRef.current.get(metadata.id) + ); } return metadata; }, diff --git a/src/browser/features/ChatInput/index.tsx b/src/browser/features/ChatInput/index.tsx index ff58313208..1223a703d1 100644 --- a/src/browser/features/ChatInput/index.tsx +++ b/src/browser/features/ChatInput/index.tsx @@ -48,10 +48,6 @@ import { } from "@/browser/utils/additionalSystemContextStore"; import { useSendMessageOptions } from "@/browser/hooks/useSendMessageOptions"; import { setWorkspaceModelWithOrigin } from "@/browser/utils/modelChange"; -import { - clearPendingWorkspaceAiSettings, - markPendingWorkspaceAiSettings, -} from "@/browser/utils/workspaceAiSettingsSync"; import { resolveWorkspaceAiSettingsForAgent } from "@/browser/utils/workspaceModeAi"; import { getModelKey, @@ -681,43 +677,13 @@ const ChatInputInner: React.FC = (props) => { prev && typeof prev === "object" ? prev : {}; return { ...record, - // Include reasoningMode so a model change cannot wipe the persisted - // pro-mode choice (backend replaces the agent's settings wholesale). [normalizedAgentId]: { model: selectedModel, thinkingLevel, reasoningMode }, }; }, {} ); - - // Workspace variant: persist to backend for cross-device consistency. - if (!api) { - return; - } - - markPendingWorkspaceAiSettings(workspaceId, normalizedAgentId, { - model: selectedModel, - thinkingLevel, - reasoningMode, - }); - - api.workspace - .updateAgentAISettings({ - workspaceId, - agentId: normalizedAgentId, - aiSettings: { model: selectedModel, thinkingLevel, reasoningMode }, - }) - .then((result) => { - if (!result.success) { - clearPendingWorkspaceAiSettings(workspaceId, normalizedAgentId); - } - }) - .catch(() => { - clearPendingWorkspaceAiSettings(workspaceId, normalizedAgentId); - // Best-effort only. If offline or backend is old, sendMessage will persist. - }); }, [ - api, agentId, creationParentProjectPath, ensureModelInSettings, diff --git a/src/browser/utils/workspaceAiSettingsSync.ts b/src/browser/utils/workspaceAiSettingsSync.ts index 69beb19169..4a0c89ca07 100644 --- a/src/browser/utils/workspaceAiSettingsSync.ts +++ b/src/browser/utils/workspaceAiSettingsSync.ts @@ -2,13 +2,6 @@ import { normalizeModelPreference } from "@/browser/utils/messages/buildSendMess import type { OpenAIReasoningMode, ThinkingLevel } from "@/common/types/thinking"; import type { FrontendWorkspaceMetadata } from "@/common/types/workspace"; -interface WorkspaceAiSettingsSnapshot { - model: string; - thinkingLevel: ThinkingLevel; - /** Optional: legacy settings (and non-OpenAI workflows) omit it. */ - reasoningMode?: OpenAIReasoningMode; -} - export function getWorkspaceAiSettingsFromMetadata( metadata: FrontendWorkspaceMetadata | undefined, agentId: string | undefined @@ -36,55 +29,3 @@ export function resolveEffectiveComposerModel( // Match ChatInput precedence so shortcuts and palette actions gate on the model users see. return normalizeModelPreference(preferredModel, metadataModel ?? defaultModel); } - -const pendingAiSettingsByWorkspace = new Map(); - -function getPendingKey(workspaceId: string, agentId: string): string { - return `${workspaceId}:${agentId}`; -} - -export function markPendingWorkspaceAiSettings( - workspaceId: string, - agentId: string, - settings: WorkspaceAiSettingsSnapshot -): void { - if (!workspaceId || !agentId) { - return; - } - pendingAiSettingsByWorkspace.set(getPendingKey(workspaceId, agentId), settings); -} - -export function clearPendingWorkspaceAiSettings(workspaceId: string, agentId: string): void { - if (!workspaceId || !agentId) { - return; - } - pendingAiSettingsByWorkspace.delete(getPendingKey(workspaceId, agentId)); -} - -export function shouldApplyWorkspaceAiSettingsFromBackend( - workspaceId: string, - agentId: string, - incoming: WorkspaceAiSettingsSnapshot -): boolean { - if (!workspaceId || !agentId) { - return true; - } - - const key = getPendingKey(workspaceId, agentId); - const pending = pendingAiSettingsByWorkspace.get(key); - if (!pending) { - return true; - } - - const matches = - pending.model === incoming.model && - pending.thinkingLevel === incoming.thinkingLevel && - // Absent reasoningMode is semantically "standard" on both sides. - (pending.reasoningMode ?? "standard") === (incoming.reasoningMode ?? "standard"); - if (matches) { - pendingAiSettingsByWorkspace.delete(key); - return true; - } - - return false; -} diff --git a/src/browser/utils/workspaceModeAi.test.ts b/src/browser/utils/workspaceModeAi.test.ts index 30dcae0fe3..596c872abb 100644 --- a/src/browser/utils/workspaceModeAi.test.ts +++ b/src/browser/utils/workspaceModeAi.test.ts @@ -40,7 +40,7 @@ describe("resolveWorkspaceAiSettingsForAgent", () => { test("uses workspace-by-agent fallback when explicitly enabled", () => { const result = resolveWorkspaceAiSettingsForAgent({ agentId: "exec", - agentAiDefaults: {}, + agentAiDefaults: { exec: { modelString: "openai:gpt-5.3-codex", thinkingLevel: "high" } }, workspaceByAgent: { exec: { model: "openai:gpt-5.2", thinkingLevel: "medium" }, }, @@ -60,7 +60,7 @@ describe("resolveWorkspaceAiSettingsForAgent", () => { test("ignores workspace-by-agent fallback when disabled", () => { const result = resolveWorkspaceAiSettingsForAgent({ agentId: "exec", - agentAiDefaults: {}, + agentAiDefaults: { exec: { modelString: "openai:gpt-5.3-codex", thinkingLevel: "high" } }, workspaceByAgent: { exec: { model: "openai:gpt-5.2", thinkingLevel: "medium" }, }, @@ -370,6 +370,28 @@ describe("resolveWorkspaceAiSettingsForAgent", () => { }); }); + test.each([undefined, "", "bogus", 42, "openai:gpt-5.2"])( + "invalid cached fields use configured defaults (%s)", + (model) => { + const result = resolveWorkspaceAiSettingsForAgent({ + agentId: "exec", + agentAiDefaults: { exec: { modelString: "openai:gpt-5.2", thinkingLevel: "high" } }, + workspaceByAgent: { + exec: { + model: model as string, + thinkingLevel: "invalid" as ThinkingLevel, + }, + }, + useWorkspaceByAgentFallback: true, + fallbackModel: "openai:gpt-5.2-mini", + existingModel: "anthropic:claude-opus-4-6", + existingThinking: "off", + }); + expect(result.resolvedModel).toBe("openai:gpt-5.2"); + expect(result.resolvedThinking).toBe("high"); + } + ); + test("guards non-string persisted model values", () => { const result = resolveWorkspaceAiSettingsForAgent({ agentId: "exec", diff --git a/src/browser/utils/workspaceModeAi.ts b/src/browser/utils/workspaceModeAi.ts index 24f52a9273..488078230b 100644 --- a/src/browser/utils/workspaceModeAi.ts +++ b/src/browser/utils/workspaceModeAi.ts @@ -1,4 +1,5 @@ import type { AgentAiDefaults } from "@/common/types/agentAiDefaults"; +import { isValidModelFormat } from "@/common/utils/ai/models"; import type { AiSettingSource } from "@/common/types/agentAiSettings"; import { coerceOpenAIReasoningMode, @@ -87,11 +88,11 @@ export function resolveWorkspaceAiSettingsForAgent(args: { args.agentAiDefaults, args.agentBaseById ); - const configuredModel = configuredDefaults.modelString; - const workspaceOverrideModel = - args.useWorkspaceByAgentFallback && typeof workspaceOverride?.model === "string" - ? workspaceOverride.model - : undefined; + const cachedModel = + typeof workspaceOverride?.model === "string" ? workspaceOverride.model.trim() : ""; + const workspaceModel = isValidModelFormat(cachedModel) ? cachedModel : undefined; + const configuredModel = workspaceModel ? undefined : configuredDefaults.modelString; + const workspaceOverrideModel = args.useWorkspaceByAgentFallback ? workspaceModel : undefined; const inheritedModelCandidate = workspaceOverrideModel ?? (typeof args.existingModel === "string" ? args.existingModel : undefined) ?? @@ -106,11 +107,15 @@ export function resolveWorkspaceAiSettingsForAgent(args: { // Persisted workspace settings can be stale/corrupt; re-validate inherited values // so mode sync keeps self-healing behavior instead of propagating invalid options. + const workspaceThinking = coerceThinkingLevel(workspaceOverride?.thinkingLevel); const workspaceOverrideThinking = args.useWorkspaceByAgentFallback - ? coerceThinkingLevel(workspaceOverride?.thinkingLevel) + ? workspaceThinking : undefined; const inheritedThinking = workspaceOverrideThinking ?? coerceThinkingLevel(args.existingThinking); - const resolvedThinking = configuredDefaults.thinkingLevel ?? inheritedThinking ?? "off"; + const resolvedThinking = + (workspaceThinking != null ? undefined : configuredDefaults.thinkingLevel) ?? + inheritedThinking ?? + "off"; // An existing per-agent bucket owns the reasoning choice outright (matching // targetWorkspaceBucketToLayer): a configured Pro default must not re-inject diff --git a/src/node/acp/agent.ts b/src/node/acp/agent.ts index ec10041fbb..02bd98ff12 100644 --- a/src/node/acp/agent.ts +++ b/src/node/acp/agent.ts @@ -49,7 +49,7 @@ import { } from "@/common/utils/subProjects"; import { createAsyncMessageQueue } from "@/common/utils/asyncMessageQueue"; import { negotiateCapabilities, type NegotiatedCapabilities } from "./capabilities"; -import { AGENT_MODE_CONFIG_ID, buildConfigOptions, handleSetConfigOption } from "./configOptions"; +import { buildConfigOptions, handleSetConfigOption } from "./configOptions"; import { forkSessionFromWorkspace } from "./experimental/sessionFork"; import { canonicalizePathForWorkspaceMatch, @@ -372,7 +372,6 @@ export class MuxAgent implements Agent { const agentId = meta.agentId ?? workspace.agentId ?? DEFAULT_AGENT_ID; const aiSettings = await resolveAgentAiSettings(this.server.client, agentId, workspaceId); - await this.persistAiSettings(workspaceId, agentId, aiSettings); this.sessionStateById.set(sessionId, { workspaceId, @@ -388,6 +387,7 @@ export class MuxAgent implements Agent { sessionId, configOptions: await buildConfigOptions(this.server.client, workspaceId, { activeAgentId: agentId, + aiSettings, }), }; @@ -551,8 +551,6 @@ export class MuxAgent implements Agent { meta.forkName ); - await this.persistAiSettings(forked.workspaceId, forked.agentId, forked.aiSettings); - this.sessionStateById.set(forked.sessionId, { workspaceId: forked.workspaceId, runtimeMode: forked.runtimeMode, @@ -598,8 +596,7 @@ export class MuxAgent implements Agent { options: { model: sessionState.aiSettings.model, thinkingLevel: sessionState.aiSettings.thinkingLevel, - // Per-workspace pro mode from workspace metadata; the send path - // re-gates per model/route so this is inert for unsupported models. + // The send path re-gates pro mode for the selected model and route. reasoningMode: sessionState.aiSettings.reasoningMode, agentId: sessionState.agentId, }, @@ -655,24 +652,21 @@ export class MuxAgent implements Agent { ); } - const activeAgentId = this.sessionStateById.get(sessionId)?.agentId; + const sessionState = this.sessionStateById.get(sessionId); const configOptions = await handleSetConfigOption( this.server.client, workspaceId, params.configId, params.value, { - activeAgentId, + activeAgentId: sessionState?.agentId, + aiSettings: sessionState?.aiSettings, onAgentModeChanged: (agentId, aiSettings) => { this.updateSessionAgentState(sessionId, agentId, aiSettings); }, } ); - if (trimmedConfigId !== AGENT_MODE_CONFIG_ID) { - await this.refreshSessionState(sessionId); - } - return { configOptions }; } @@ -2143,7 +2137,9 @@ export class MuxAgent implements Agent { // selection lives in sessionStateById and must not be reverted by a // workspace.agentId value from the backend. const agentId = existing?.agentId ?? workspace.agentId ?? DEFAULT_AGENT_ID; + // Picker choices remain session-local until the next user message sends them. const aiSettings = + existing?.aiSettings ?? workspace.aiSettingsByAgent?.[agentId] ?? workspace.aiSettings ?? (await resolveAgentAiSettings(this.server.client, agentId, workspaceId)); @@ -2159,36 +2155,6 @@ export class MuxAgent implements Agent { return nextState; } - private async persistAiSettings( - workspaceId: string, - agentId: string, - aiSettings: ResolvedAiSettings - ): Promise { - if (agentId === "plan" || agentId === "exec") { - const updateModeResult = await this.server.client.workspace.updateModeAISettings({ - workspaceId, - mode: agentId, - aiSettings, - }); - - if (!updateModeResult.success) { - throw new Error(`workspace.updateModeAISettings failed: ${updateModeResult.error}`); - } - - return; - } - - const updateAgentResult = await this.server.client.workspace.updateAgentAISettings({ - workspaceId, - agentId, - aiSettings, - }); - - if (!updateAgentResult.success) { - throw new Error(`workspace.updateAgentAISettings failed: ${updateAgentResult.error}`); - } - } - async waitForDisconnectCleanup(): Promise { await this.disconnectCleanupPromise; } diff --git a/src/node/acp/configOptions.ts b/src/node/acp/configOptions.ts index 698bc6a3ec..06ab72f25e 100644 --- a/src/node/acp/configOptions.ts +++ b/src/node/acp/configOptions.ts @@ -1,4 +1,5 @@ import assert from "node:assert/strict"; +import { isValidModelFormat, normalizeSelectedModel } from "@/common/utils/ai/models"; import type { SessionConfigOption, SessionConfigSelectOption } from "@agentclientprotocol/sdk"; import { KNOWN_MODELS } from "@/common/constants/knownModels"; import type { AgentDefinitionFrontmatter } from "@/common/types/agentDefinition"; @@ -127,29 +128,15 @@ async function resolveAvailableAgentIds( } type WorkspaceInfo = NonNullable>>; -type UpdateAgentAiSettingsResult = Awaited< - ReturnType ->; - interface BuildConfigOptionsArgs { activeAgentId?: string; + aiSettings?: ResolvedAiSettings; } -interface HandleSetConfigOptionArgs { - activeAgentId?: string; +interface HandleSetConfigOptionArgs extends BuildConfigOptionsArgs { onAgentModeChanged?: (agentId: string, aiSettings: ResolvedAiSettings) => Promise | void; } -function isModeAgentId(agentId: string): agentId is "plan" | "exec" { - return agentId === "plan" || agentId === "exec"; -} - -function ensureUpdateSucceeded(result: UpdateAgentAiSettingsResult, operation: string): void { - if (!result.success) { - throw new Error(`${operation} failed: ${result.error}`); - } -} - async function getWorkspaceInfoOrThrow( client: ORPCClient, workspaceId: string @@ -239,30 +226,6 @@ function buildThinkingLevelSelectOptions(modelString: string): SessionConfigSele })); } -async function persistAgentAiSettings( - client: ORPCClient, - workspaceId: string, - agentId: string, - aiSettings: ResolvedAiSettings -): Promise { - if (isModeAgentId(agentId)) { - const updateModeResult = await client.workspace.updateModeAISettings({ - workspaceId, - mode: agentId, - aiSettings, - }); - ensureUpdateSucceeded(updateModeResult, "workspace.updateModeAISettings"); - return; - } - - const updateAgentResult = await client.workspace.updateAgentAISettings({ - workspaceId, - agentId, - aiSettings, - }); - ensureUpdateSucceeded(updateAgentResult, "workspace.updateAgentAISettings"); -} - export async function buildConfigOptions( client: ORPCClient, workspaceId: string, @@ -282,12 +245,9 @@ export async function buildConfigOptions( : getCurrentAgentId(workspace), availableAgentIds.length > 0 ? availableAgentIds : exposedAgentModes.map((mode) => mode.value) ); - const currentAiSettings = await resolveCurrentAiSettings( - client, - workspace, - workspaceId, - currentAgentId - ); + const currentAiSettings = + args?.aiSettings ?? + (await resolveCurrentAiSettings(client, workspace, workspaceId, currentAgentId)); const agentModeOptions = buildAgentModeSelectOptions(exposedAgentModes, currentAgentId); const effectiveThinkingLevel = enforceThinkingPolicy( @@ -355,13 +315,17 @@ export async function handleSetConfigOption( knownAgentIds ); + let nextAgentId = currentAgentId; + let nextAiSettings: ResolvedAiSettings; if (trimmedConfigId === AGENT_MODE_CONFIG_ID) { - const nextAgentId = resolveCurrentAgentId(trimmedValue, knownAgentIds); + nextAgentId = resolveCurrentAgentId(trimmedValue, knownAgentIds); // Prefer workspace-specific settings already saved for the target agent // (e.g., user customized model/thinking for this mode). Only fall back // to resolved defaults when no prior settings exist for the agent. - const existingSettings = workspace.aiSettingsByAgent?.[nextAgentId]; + const existingSettings = + (nextAgentId === currentAgentId ? args?.aiSettings : undefined) ?? + workspace.aiSettingsByAgent?.[nextAgentId]; const resolvedAiSettings = existingSettings?.model != null && existingSettings?.thinkingLevel != null ? { @@ -371,7 +335,7 @@ export async function handleSetConfigOption( } : await resolveAgentAiSettings(client, nextAgentId, trimmedWorkspaceId); - const normalizedAiSettings: ResolvedAiSettings = { + nextAiSettings = { model: resolvedAiSettings.model, thinkingLevel: enforceThinkingPolicy( resolvedAiSettings.model, @@ -381,60 +345,40 @@ export async function handleSetConfigOption( ? { reasoningMode: resolvedAiSettings.reasoningMode } : {}), }; - - await persistAgentAiSettings(client, trimmedWorkspaceId, nextAgentId, normalizedAiSettings); - if (args?.onAgentModeChanged != null) { - await args.onAgentModeChanged(nextAgentId, normalizedAiSettings); + } else { + const currentAiSettings = + args?.aiSettings ?? + (await resolveCurrentAiSettings(client, workspace, trimmedWorkspaceId, currentAgentId)); + + if (trimmedConfigId === MODEL_CONFIG_ID) { + const model = normalizeSelectedModel(trimmedValue).trim(); + if (!isValidModelFormat(model)) { + throw new Error(`Invalid model format: ${trimmedValue}`); + } + // The send path re-gates pro mode for the selected model and route. + nextAiSettings = { + ...currentAiSettings, + model, + thinkingLevel: enforceThinkingPolicy(model, currentAiSettings.thinkingLevel), + }; + } else if (trimmedConfigId === THINKING_LEVEL_CONFIG_ID) { + if (!isThinkingLevel(trimmedValue)) { + throw new Error( + `handleSetConfigOption: value must be a valid ThinkingLevel, got '${trimmedValue}'` + ); + } + nextAiSettings = { + ...currentAiSettings, + thinkingLevel: enforceThinkingPolicy(currentAiSettings.model, trimmedValue), + }; + } else { + throw new Error(`Unsupported config option id '${trimmedConfigId}'`); } - - return buildConfigOptions(client, trimmedWorkspaceId, { activeAgentId: nextAgentId }); } - const currentAiSettings = await resolveCurrentAiSettings( - client, - workspace, - trimmedWorkspaceId, - currentAgentId - ); - - if (trimmedConfigId === MODEL_CONFIG_ID) { - const clampedThinkingLevel = enforceThinkingPolicy( - trimmedValue, - currentAiSettings.thinkingLevel - ); - - // Retain pro mode across model changes (matching the settings UI); the - // send path re-gates per model so unsupported models are unaffected. - await persistAgentAiSettings(client, trimmedWorkspaceId, currentAgentId, { - model: trimmedValue, - thinkingLevel: clampedThinkingLevel, - ...(currentAiSettings.reasoningMode != null - ? { reasoningMode: currentAiSettings.reasoningMode } - : {}), - }); - - return buildConfigOptions(client, trimmedWorkspaceId, { activeAgentId: currentAgentId }); - } - - if (trimmedConfigId === THINKING_LEVEL_CONFIG_ID) { - if (!isThinkingLevel(trimmedValue)) { - throw new Error( - `handleSetConfigOption: value must be a valid ThinkingLevel, got '${trimmedValue}'` - ); - } - - const clampedThinkingLevel = enforceThinkingPolicy(currentAiSettings.model, trimmedValue); - - await persistAgentAiSettings(client, trimmedWorkspaceId, currentAgentId, { - model: currentAiSettings.model, - thinkingLevel: clampedThinkingLevel, - ...(currentAiSettings.reasoningMode != null - ? { reasoningMode: currentAiSettings.reasoningMode } - : {}), - }); - - return buildConfigOptions(client, trimmedWorkspaceId, { activeAgentId: currentAgentId }); - } - - throw new Error(`Unsupported config option id '${trimmedConfigId}'`); + await args?.onAgentModeChanged?.(nextAgentId, nextAiSettings); + return buildConfigOptions(client, trimmedWorkspaceId, { + activeAgentId: nextAgentId, + aiSettings: nextAiSettings, + }); } diff --git a/src/node/services/workspaceService.test.ts b/src/node/services/workspaceService.test.ts index 12e71b83f3..2ea3a5bc0e 100644 --- a/src/node/services/workspaceService.test.ts +++ b/src/node/services/workspaceService.test.ts @@ -8755,11 +8755,7 @@ describe("WorkspaceService sendMessage status clearing", () => { ( workspaceService as unknown as { - maybePersistAISettingsFromOptions: ( - workspaceId: string, - options: unknown, - source: "send" | "resume" - ) => Promise; + maybePersistAISettingsFromOptions: (workspaceId: string, options: unknown) => Promise; } ).maybePersistAISettingsFromOptions = mock(() => Promise.resolve()); }); @@ -8768,6 +8764,27 @@ describe("WorkspaceService sendMessage status clearing", () => { await cleanupHistory(); }); + test.each(["send", "synthetic", "resume"] as const)( + "only a user message updates remembered settings (%s)", + async (kind) => { + const persist = mock(() => Promise.resolve()); + ( + workspaceService as unknown as { + maybePersistAISettingsFromOptions: typeof persist; + } + ).maybePersistAISettingsFromOptions = persist; + const options = { model: "openai:gpt-5.2", agentId: "plan", thinkingLevel: "high" as const }; + const result = + kind === "resume" + ? await workspaceService.resumeStream("test-workspace", options) + : await workspaceService.sendMessage("test-workspace", "hello", options, { + synthetic: kind === "synthetic", + }); + expect(result.success).toBe(true); + expect(persist).toHaveBeenCalledTimes(kind === "send" ? 1 : 0); + } + ); + test("delegates manual pricing rejections to AgentSession so user input is preserved", async () => { fakeSession.isBusy.mockReturnValue(false); const pricingError: SendMessageError = { type: "unknown", raw: "unpriced model" }; @@ -8888,11 +8905,12 @@ describe("WorkspaceService sendMessage status clearing", () => { // send is supersedable: the manual send goes direct and the heartbeat's own // preflight-count skip refuses it. fakeSession.isBusy.mockReturnValue(false); - const persistSettings = ( - workspaceService as unknown as { maybePersistAISettingsFromOptions: ReturnType } - ).maybePersistAISettingsFromOptions; + const pricingGate = mock(() => Promise.resolve(Ok(undefined))); + workspaceService.setWorkspaceGoalService({ + assertPricedModelForBudgetedGoal: pricingGate, + } as unknown as WorkspaceGoalService); const heartbeatPreflight = createDeferred(); - persistSettings.mockImplementationOnce(() => heartbeatPreflight.promise); + pricingGate.mockImplementationOnce(() => heartbeatPreflight.promise.then(() => Ok(undefined))); const sendOptions = { model: "openai:gpt-4o-mini", agentId: "exec" }; const heartbeatResult = workspaceService.sendMessage( @@ -8905,7 +8923,7 @@ describe("WorkspaceService sendMessage status clearing", () => { requireIdle: true, } ); - await waitForCondition(() => persistSettings.mock.calls.length === 1); + await waitForCondition(() => pricingGate.mock.calls.length === 1); const manualSend = createDeferred>(); fakeSession.sendMessage.mockImplementationOnce(() => manualSend.promise); @@ -8932,19 +8950,22 @@ describe("WorkspaceService sendMessage status clearing", () => { "test-workspace", fakeSession as unknown as AgentSession ); - const persistSettings = ( - workspaceService as unknown as { maybePersistAISettingsFromOptions: ReturnType } - ).maybePersistAISettingsFromOptions; + const pricingGate = mock(() => Promise.resolve(Ok(undefined))); + workspaceService.setWorkspaceGoalService({ + assertPricedModelForBudgetedGoal: pricingGate, + } as unknown as WorkspaceGoalService); const sendOptions = { model: "openai:gpt-4o-mini", agentId: "exec" }; const maintenancePreflight = createDeferred(); - persistSettings.mockImplementationOnce(() => maintenancePreflight.promise); + pricingGate.mockImplementationOnce(() => + maintenancePreflight.promise.then(() => Ok(undefined)) + ); const maintenanceResult = workspaceService.sendMessage( "test-workspace", "check in", sendOptions, { synthetic: true, agentInitiated: true, requireIdle: true } ); - await waitForCondition(() => persistSettings.mock.calls.length === 1); + await waitForCondition(() => pricingGate.mock.calls.length === 1); const firstManual = createDeferred>(); fakeSession.sendMessage.mockImplementationOnce(() => firstManual.promise); @@ -9007,11 +9028,12 @@ describe("WorkspaceService sendMessage status clearing", () => { // requireIdle. The manual send must not queue behind the heartbeat, and the heartbeat // must not start once that input is in preflight; its next slot fires anyway. fakeSession.isBusy.mockReturnValue(false); - const persistSettings = ( - workspaceService as unknown as { maybePersistAISettingsFromOptions: ReturnType } - ).maybePersistAISettingsFromOptions; + const pricingGate = mock(() => Promise.resolve(Ok(undefined))); + workspaceService.setWorkspaceGoalService({ + assertPricedModelForBudgetedGoal: pricingGate, + } as unknown as WorkspaceGoalService); const heartbeatPreflight = createDeferred(); - persistSettings.mockImplementationOnce(() => heartbeatPreflight.promise); + pricingGate.mockImplementationOnce(() => heartbeatPreflight.promise.then(() => Ok(undefined))); const sendOptions = { model: "openai:gpt-4o-mini", agentId: "exec" }; const heartbeatResult = workspaceService.sendMessage( @@ -9026,7 +9048,7 @@ describe("WorkspaceService sendMessage status clearing", () => { yieldToQueuedMessages: true, } ); - await waitForCondition(() => persistSettings.mock.calls.length === 1); + await waitForCondition(() => pricingGate.mock.calls.length === 1); const manualSend = createDeferred>(); fakeSession.sendMessage.mockImplementationOnce(() => manualSend.promise); @@ -9865,11 +9887,7 @@ describe("WorkspaceService pending auto-title", () => { ( workspaceService as unknown as { - maybePersistAISettingsFromOptions: ( - workspaceId: string, - options: unknown, - source: "send" | "resume" - ) => Promise; + maybePersistAISettingsFromOptions: (workspaceId: string, options: unknown) => Promise; } ).maybePersistAISettingsFromOptions = mock(() => Promise.resolve()); }); @@ -12049,26 +12067,18 @@ describe("WorkspaceService maybePersistAISettingsFromOptions", () => { const persistSpy = mock(() => Promise.resolve({ success: true as const, data: true })); interface WorkspaceServiceTestAccess { - maybePersistAISettingsFromOptions: ( - workspaceId: string, - options: unknown, - context: "send" | "resume" - ) => Promise; + maybePersistAISettingsFromOptions: (workspaceId: string, options: unknown) => Promise; persistWorkspaceAISettingsForAgent: (...args: unknown[]) => unknown; } const svc = workspaceService as unknown as WorkspaceServiceTestAccess; svc.persistWorkspaceAISettingsForAgent = persistSpy; - await svc.maybePersistAISettingsFromOptions( - "ws", - { - agentId: "reviewer", - model: "openai:gpt-4o-mini", - thinkingLevel: "off", - }, - "send" - ); + await svc.maybePersistAISettingsFromOptions("ws", { + agentId: "reviewer", + model: "openai:gpt-4o-mini", + thinkingLevel: "off", + }); expect(persistSpy).toHaveBeenCalledTimes(1); }); @@ -12077,26 +12087,18 @@ describe("WorkspaceService maybePersistAISettingsFromOptions", () => { const persistSpy = mock(() => Promise.resolve({ success: true as const, data: true })); interface WorkspaceServiceTestAccess { - maybePersistAISettingsFromOptions: ( - workspaceId: string, - options: unknown, - context: "send" | "resume" - ) => Promise; + maybePersistAISettingsFromOptions: (workspaceId: string, options: unknown) => Promise; persistWorkspaceAISettingsForAgent: (...args: unknown[]) => unknown; } const svc = workspaceService as unknown as WorkspaceServiceTestAccess; svc.persistWorkspaceAISettingsForAgent = persistSpy; - await svc.maybePersistAISettingsFromOptions( - "ws", - { - agentId: "exec", - model: "openai:gpt-4o-mini", - thinkingLevel: "off", - }, - "send" - ); + await svc.maybePersistAISettingsFromOptions("ws", { + agentId: "exec", + model: "openai:gpt-4o-mini", + thinkingLevel: "off", + }); expect(persistSpy).toHaveBeenCalledTimes(1); }); @@ -12105,11 +12107,7 @@ describe("WorkspaceService maybePersistAISettingsFromOptions", () => { const persistSpy = mock(() => Promise.resolve({ success: true as const, data: true })); interface WorkspaceServiceTestAccess { - maybePersistAISettingsFromOptions: ( - workspaceId: string, - options: unknown, - context: "send" | "resume" - ) => Promise; + maybePersistAISettingsFromOptions: (workspaceId: string, options: unknown) => Promise; persistWorkspaceAISettingsForAgent: (...args: unknown[]) => unknown; config: { findWorkspace: ( @@ -12147,15 +12145,11 @@ describe("WorkspaceService maybePersistAISettingsFromOptions", () => { ]), })); - await svc.maybePersistAISettingsFromOptions( - "ws", - { - agentId: "exec", - model: "openai:gpt-4o-mini", - thinkingLevel: "off", - }, - "send" - ); + await svc.maybePersistAISettingsFromOptions("ws", { + agentId: "exec", + model: "openai:gpt-4o-mini", + thinkingLevel: "off", + }); expect(persistSpy).toHaveBeenCalledTimes(1); expect(persistSpy).toHaveBeenCalledWith( @@ -13733,7 +13727,6 @@ describe("WorkspaceService reorderPinned across projects", () => { const mockConfig: Partial = { srcDir: "/tmp/src", - getSessionDir: mock(() => "/tmp/test/sessions"), findWorkspace: mock((id: string) => { const found = findEntry(id); if (!found) return null; diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index c39d187e7d..0ebe0fa36c 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -9476,14 +9476,9 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { ); } - /** - * Best-effort persist AI settings from send/resume options. - * Skips requests explicitly marked to avoid persistence. - */ private async maybePersistAISettingsFromOptions( workspaceId: string, - options: SendMessageOptions | undefined, - context: "send" | "resume" + options: SendMessageOptions | undefined ): Promise { if (options?.skipAiSettingsPersistence) { // One-shot/compaction sends shouldn't overwrite workspace defaults. @@ -9499,14 +9494,13 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { agentId, extractedSettings, { - // Normal sends/resumes also persist the selected agent so future backend heartbeat - // dispatches can reuse the same workspace default after reloads and reconnects. + // Save the selected agent so heartbeats can reuse it after reloads and reconnects. persistSelectedAgentId: true, ...(options?.disableWorkspaceAgents === true ? { disableWorkspaceAgents: true } : {}), } ); if (!persistResult.success) { - log.debug(`Failed to persist workspace AI settings from ${context} options`, { + log.debug("Failed to persist workspace AI settings from user message", { workspaceId, error: persistResult.error, }); @@ -10910,8 +10904,10 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { return Err(pricingGate.error); } - // Persist last-used model + thinking level for cross-device consistency. - await this.maybePersistAISettingsFromOptions(workspaceId, normalizedOptions, "send"); + // Synthetic turns must not replace the user's remembered model and mode. + if (internal?.synthetic !== true) { + await this.maybePersistAISettingsFromOptions(workspaceId, normalizedOptions); + } // Decide queue-or-direct in arrival order: a later send whose awaits above finished // first would otherwise enqueue ahead of an earlier one. The decision below runs @@ -11415,9 +11411,6 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { return Err(pricingGate.error); } - // Persist last-used model + thinking level for cross-device consistency. - await this.maybePersistAISettingsFromOptions(workspaceId, normalizedOptions, "resume"); - // Non-destructive interrupt cascades preserve descendant task workspaces with // taskStatus=interrupted. Transition before stream start so task orchestration stream-end // handling does not early-return on interrupted status. diff --git a/tests/ipc/acp.configOptions.test.ts b/tests/ipc/acp.configOptions.test.ts index baa1cb71fe..1b73aed0a6 100644 --- a/tests/ipc/acp.configOptions.test.ts +++ b/tests/ipc/acp.configOptions.test.ts @@ -57,6 +57,7 @@ function createHarness( ): { client: ORPCClient; getWorkspaceState: () => WorkspaceState; + onAgentModeChanged: jest.Mock; updateModeCalls: Array<{ workspaceId: string; mode: "exec" | "plan"; @@ -68,7 +69,7 @@ function createHarness( aiSettings: WorkspaceAiSettings; }>; } { - let workspaceState: WorkspaceState = { + const workspaceState: WorkspaceState = { agentId: initial.agentId, aiSettings: { ...initial.aiSettings }, aiSettingsByAgent: { ...initial.aiSettingsByAgent }, @@ -108,16 +109,6 @@ function createHarness( }) => { updateModeCalls.push(input); - workspaceState = { - ...workspaceState, - agentId: input.mode, - aiSettings: { ...input.aiSettings }, - aiSettingsByAgent: { - ...workspaceState.aiSettingsByAgent, - [input.mode]: { ...input.aiSettings }, - }, - }; - return { success: true as const, data: undefined }; }, updateAgentAISettings: async (input: { @@ -127,16 +118,6 @@ function createHarness( }) => { updateAgentCalls.push(input); - workspaceState = { - ...workspaceState, - agentId: input.agentId, - aiSettings: { ...input.aiSettings }, - aiSettingsByAgent: { - ...workspaceState.aiSettingsByAgent, - [input.agentId]: { ...input.aiSettings }, - }, - }; - return { success: true as const, data: undefined }; }, }, @@ -148,6 +129,7 @@ function createHarness( return { client, getWorkspaceState: () => workspaceState, + onAgentModeChanged: jest.fn(), updateModeCalls, updateAgentCalls, }; @@ -261,10 +243,11 @@ describe("ACP config options", () => { await handleSetConfigOption(harness.client, "ws-1", AGENT_MODE_CONFIG_ID, "exec", { activeAgentId: "plan", + onAgentModeChanged: harness.onAgentModeChanged, }); - expect(harness.updateModeCalls).toHaveLength(1); - expect(harness.updateModeCalls[0]?.aiSettings.reasoningMode).toBe("pro"); + expect(harness.updateModeCalls).toHaveLength(0); + expect(harness.onAgentModeChanged.mock.calls[0]?.[1].reasoningMode).toBe("pro"); }); it("preserves pro reasoning mode across model and thinking level changes", async () => { @@ -278,16 +261,42 @@ describe("ACP config options", () => { await handleSetConfigOption(harness.client, "ws-1", "model", "anthropic:claude-opus-4-6", { activeAgentId: "exec", + onAgentModeChanged: harness.onAgentModeChanged, }); - expect(harness.updateModeCalls[0]?.aiSettings.reasoningMode).toBe("pro"); + expect(harness.onAgentModeChanged.mock.calls[0]?.[1].reasoningMode).toBe("pro"); await handleSetConfigOption(harness.client, "ws-1", "thinkingLevel", "medium", { activeAgentId: "exec", + aiSettings: harness.onAgentModeChanged.mock.calls[0]?.[1], + onAgentModeChanged: harness.onAgentModeChanged, + }); + expect(harness.onAgentModeChanged.mock.calls[1]?.[1]).toEqual({ + model: "anthropic:claude-opus-4-6", + thinkingLevel: "medium", + reasoningMode: "pro", }); - expect(harness.updateModeCalls[1]?.aiSettings.reasoningMode).toBe("pro"); + expect(harness.updateModeCalls).toHaveLength(0); + expect(harness.updateAgentCalls).toHaveLength(0); }); - it("clamps persisted thinking level when model changes", async () => { + it.each(["bogus", "openai:", ":gpt-5.2"])( + "rejects malformed local model choices (%s)", + async (model) => { + const harness = createHarness({ + agentId: "exec", + aiSettings: { model: "openai:gpt-5.2", thinkingLevel: "off" }, + aiSettingsByAgent: {}, + }); + await expect( + handleSetConfigOption(harness.client, "ws-1", "model", model, { + onAgentModeChanged: harness.onAgentModeChanged, + }) + ).rejects.toThrow(); + expect(harness.onAgentModeChanged).not.toHaveBeenCalled(); + } + ); + + it("clamps local thinking level when model changes", async () => { const harness = createHarness({ agentId: "exec", aiSettings: { @@ -307,11 +316,11 @@ describe("ACP config options", () => { "ws-1", "model", "openai:gpt-5-pro", - { activeAgentId: "exec" } + { activeAgentId: "exec", onAgentModeChanged: harness.onAgentModeChanged } ); - expect(harness.updateModeCalls).toHaveLength(1); - expect(harness.updateModeCalls[0]?.aiSettings).toEqual({ + expect(harness.updateModeCalls).toHaveLength(0); + expect(harness.onAgentModeChanged.mock.calls[0]?.[1]).toEqual({ model: "openai:gpt-5-pro", thinkingLevel: "high", }); @@ -322,8 +331,8 @@ describe("ACP config options", () => { expect(thinkingOption.currentValue).toBe("high"); expect(thinkingEntries.map((entry) => entry.value)).toEqual(["high"]); expect(harness.getWorkspaceState().aiSettingsByAgent.exec).toEqual({ - model: "openai:gpt-5-pro", - thinkingLevel: "high", + model: "anthropic:claude-opus-4-6", + thinkingLevel: "xhigh", }); }); @@ -346,10 +355,12 @@ describe("ACP config options", () => { const agentModeOption = getSelectConfigOption(options, AGENT_MODE_CONFIG_ID); expect(agentModeOption.currentValue).toBe("exec"); - const updated = await handleSetConfigOption(harness.client, "ws-1", "thinkingLevel", "off"); + const updated = await handleSetConfigOption(harness.client, "ws-1", "thinkingLevel", "off", { + onAgentModeChanged: harness.onAgentModeChanged, + }); - expect(harness.updateModeCalls).toHaveLength(1); - expect(harness.updateModeCalls[0]?.mode).toBe("exec"); + expect(harness.updateModeCalls).toHaveLength(0); + expect(harness.onAgentModeChanged.mock.calls[0]?.[0]).toBe("exec"); const updatedThinkingOption = getSelectConfigOption(updated, "thinkingLevel"); expect(updatedThinkingOption.currentValue).toBe("off"); @@ -389,10 +400,12 @@ describe("ACP config options", () => { const agentModeOption = getSelectConfigOption(options, AGENT_MODE_CONFIG_ID); expect(agentModeOption.currentValue).toBe("ask"); - const updated = await handleSetConfigOption(harness.client, "ws-1", "thinkingLevel", "off"); + const updated = await handleSetConfigOption(harness.client, "ws-1", "thinkingLevel", "off", { + onAgentModeChanged: harness.onAgentModeChanged, + }); - expect(harness.updateAgentCalls).toHaveLength(1); - expect(harness.updateAgentCalls[0]?.agentId).toBe("ask"); + expect(harness.updateAgentCalls).toHaveLength(0); + expect(harness.onAgentModeChanged.mock.calls[0]?.[0]).toBe("ask"); const updatedThinkingOption = getSelectConfigOption(updated, "thinkingLevel"); expect(updatedThinkingOption.currentValue).toBe("off"); diff --git a/tests/ipc/acp.promptCorrelation.test.ts b/tests/ipc/acp.promptCorrelation.test.ts index 9109ade941..49b1f0ce25 100644 --- a/tests/ipc/acp.promptCorrelation.test.ts +++ b/tests/ipc/acp.promptCorrelation.test.ts @@ -565,6 +565,35 @@ function streamEnd( } describe("ACP prompt stream correlation", () => { + it("sends session-local picker settings despite unchanged workspace metadata", async () => { + const harness = createHarness(); + await initializeDefaultAgent(harness); + const { sessionId } = await createDefaultSession(harness); + for (const [configId, value] of [ + ["agentMode", "plan"], + ["model", "openai:gpt-5.2"], + ["thinkingLevel", "high"], + ["agentMode", "plan"], + ]) { + await harness.agent.setSessionConfigOption({ sessionId, configId, value }); + } + expect(harness.sendMessageCalls).toHaveLength(0); + + const { promptPromise, promptCorrelationId } = await startPromptTurn(harness, sessionId); + expect(harness.sendMessageCalls[0]?.options).toMatchObject({ + agentId: "plan", + model: "openai:gpt-5.2", + thinkingLevel: "high", + }); + harness.pushChatEvent( + streamStart(sessionId, "assistant-local", { acpPromptId: promptCorrelationId }) + ); + harness.pushChatEvent(streamEnd(sessionId, "assistant-local")); + await expect(promptPromise).resolves.toMatchObject({ stopReason: "end_turn" }); + harness.closeConnection(); + await harness.connectionClosed; + }); + it("ignores unrelated stream-start/end pairs while waiting for this prompt turn", async () => { const harness = createHarness(); const { newSessionResponse, promptPromise, promptCorrelationId } =