diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 33d0f97becb..381c6df8b7b 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -198,6 +198,7 @@ jobs: # when sharing the monolithic process; keep them in fresh isolated processes. src/node/services/workflows/WorkflowRunner.test.ts src/node/services/ptc/quickjsRuntime.test.ts + src/node/services/tools/code_execution.test.ts src/node/services/sandbox/sandboxHostService.test.ts src/node/services/agentPlugins/hookService.test.ts src/node/orpc/router.test.ts diff --git a/src/browser/App.tsx b/src/browser/App.tsx index 8a1befab705..26e11d83b39 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"; @@ -541,8 +537,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 @@ -574,30 +568,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); @@ -615,9 +586,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) { @@ -655,32 +624,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 aab70da5409..0c00cdf7074 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 09e899bc445..b71189df785 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 0820b9f333a..946a69141b3 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 de06288d288..df0eb2aca13 100644 --- a/src/browser/contexts/WorkspaceContext.test.tsx +++ b/src/browser/contexts/WorkspaceContext.test.tsx @@ -668,15 +668,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* () { @@ -699,19 +706,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 1c387e4e6b4..241636537fd 100644 --- a/src/browser/contexts/WorkspaceContext.tsx +++ b/src/browser/contexts/WorkspaceContext.tsx @@ -67,7 +67,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"; @@ -172,18 +171,20 @@ 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 { + // Snapshot all main-workspace choices on client load, not on navigation. + // Later metadata must not overwrite unsent choices; reload to restore backend settings. + if (metadata.parentWorkspaceId == null && previous != null) { + return; + } // Cache keyed by agentId (string) - includes exec, plan, and custom agents type WorkspaceAISettingsByAgentCache = Partial< Record< @@ -195,7 +196,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); @@ -226,17 +227,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, @@ -272,9 +262,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( @@ -1135,7 +1123,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); } @@ -1304,7 +1295,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 = @@ -1462,7 +1453,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); @@ -1850,7 +1844,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 deaa0887eab..50fd2cb3248 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, @@ -677,43 +673,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 69beb191693..4a0c89ca07f 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 30dcae0fe3f..596c872abb2 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 24f52a92734..488078230b0 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 de16ec04aa0..94ad7d67861 100644 --- a/src/node/acp/agent.ts +++ b/src/node/acp/agent.ts @@ -50,7 +50,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, @@ -373,7 +373,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, @@ -389,6 +388,7 @@ export class MuxAgent implements Agent { sessionId, configOptions: await buildConfigOptions(this.server.client, workspaceId, { activeAgentId: agentId, + aiSettings, }), }; @@ -407,16 +407,14 @@ export class MuxAgent implements Agent { async loadSession(params: LoadSessionRequest): Promise { this.assertInitialized("loadSession"); - // Pass any prior in-memory agent selection so mode switches survive - // reconnect/reload (agent mode set via set_config_option is only stored - // in ACP session state, not persisted as the workspace's active agent). + // Preserve unsent picker choices when reloading this adapter's active session. const existingState = this.sessionStateById.get(params.sessionId); const resumed = await loadSessionFromWorkspace(params, { server: this.server, sessionManager: this.sessionManager, negotiatedCapabilities: this.negotiatedCapabilities, defaultAgentId: DEFAULT_AGENT_ID, - existingSessionAgentId: existingState?.agentId, + existingSessionState: existingState, }); this.sessionStateById.set(resumed.sessionId, { @@ -499,7 +497,7 @@ export class MuxAgent implements Agent { sessionManager: this.sessionManager, negotiatedCapabilities: this.negotiatedCapabilities, defaultAgentId: DEFAULT_AGENT_ID, - existingSessionAgentId: existingState?.agentId, + existingSessionState: existingState, } ); @@ -552,8 +550,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, @@ -599,8 +595,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, }, @@ -662,24 +657,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 }; } @@ -2150,7 +2142,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)); @@ -2166,36 +2160,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 d2beb962d74..d99995313bd 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 { DEFAULT_HIDDEN_MODELS, 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 @@ -242,30 +229,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, @@ -286,12 +249,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( @@ -362,13 +322,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 ? { @@ -378,7 +342,7 @@ export async function handleSetConfigOption( } : await resolveAgentAiSettings(client, nextAgentId, trimmedWorkspaceId); - const normalizedAiSettings: ResolvedAiSettings = { + nextAiSettings = { model: resolvedAiSettings.model, thinkingLevel: enforceThinkingPolicy( resolvedAiSettings.model, @@ -388,60 +352,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/acp/experimental/sessionResume.ts b/src/node/acp/experimental/sessionResume.ts index 5752eefe43a..b4340dbc487 100644 --- a/src/node/acp/experimental/sessionResume.ts +++ b/src/node/acp/experimental/sessionResume.ts @@ -27,12 +27,8 @@ export interface SessionResumeDependencies { sessionManager: SessionManager; negotiatedCapabilities: NegotiatedCapabilities | null; defaultAgentId: string; - /** - * Agent ID from prior ACP in-memory session state (set via - * session/set_config_option mode switches). Takes precedence over - * workspace.agentId so that mode selections survive reconnect/reload. - */ - existingSessionAgentId?: string; + // Keep the active draft together when reloading an existing ACP session. + existingSessionState?: Pick; } function resolveRuntimeMode(workspace: WorkspaceInfo): RuntimeMode { @@ -127,16 +123,16 @@ export async function loadSessionFromWorkspace( deps.negotiatedCapabilities ?? undefined ); - // Prefer the ACP session's prior agent selection (from set_config_option) - // over workspace.agentId so that mode switches survive reconnect/reload. - const agentId = deps.existingSessionAgentId ?? workspace.agentId ?? deps.defaultAgentId; + const agentId = deps.existingSessionState?.agentId ?? workspace.agentId ?? deps.defaultAgentId; const aiSettings = + deps.existingSessionState?.aiSettings ?? workspace.aiSettingsByAgent?.[agentId] ?? workspace.aiSettings ?? (await resolveAgentAiSettings(deps.server.client, agentId, workspaceId)); const configOptions = await buildConfigOptions(deps.server.client, workspaceId, { activeAgentId: agentId, + aiSettings, }); return { diff --git a/src/node/services/workspaceService.test.ts b/src/node/services/workspaceService.test.ts index e1b4d86f8e1..e49d9bf1c7e 100644 --- a/src/node/services/workspaceService.test.ts +++ b/src/node/services/workspaceService.test.ts @@ -1163,7 +1163,7 @@ describe("WorkspaceService bash monitor wake reconciler wiring", () => { } ); - test.each(["options", "settings"] as const)( + test.each(["options", "pricing"] as const)( "wake yields when a turn starts during %s admission", async (gate) => { const h = await createActiveWakeHarness(); @@ -1180,11 +1180,12 @@ describe("WorkspaceService bash monitor wake reconciler wiring", () => { ); } else { const internal = h.service as unknown as { - maybePersistAISettingsFromOptions(): Promise; + assertPricedModelForBudgetedGoal(): Promise>; }; - spyOn(internal, "maybePersistAISettingsFromOptions").mockImplementationOnce(async () => { + spyOn(internal, "assertPricedModelForBudgetedGoal").mockImplementationOnce(async () => { entered.resolve(); await release.promise; + return Ok(undefined); }); } try { @@ -9843,11 +9844,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()); }); @@ -9856,6 +9853,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" }; @@ -9976,11 +9994,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( @@ -9993,7 +10012,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); @@ -10020,19 +10039,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); @@ -10095,11 +10117,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( @@ -10114,7 +10137,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); @@ -11025,11 +11048,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()); }); @@ -13209,26 +13228,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); }); @@ -13237,26 +13248,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); }); @@ -13265,11 +13268,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: ( @@ -13307,15 +13306,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( diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index d1afc4e2773..b21fdd796f2 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -9611,14 +9611,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. @@ -9634,14 +9629,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, }); @@ -11055,8 +11049,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 @@ -11578,9 +11574,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/e2e/scenarios/sidebarDragDrop.spec.ts b/tests/e2e/scenarios/sidebarDragDrop.spec.ts index b0157327249..f0cb0fe2741 100644 --- a/tests/e2e/scenarios/sidebarDragDrop.spec.ts +++ b/tests/e2e/scenarios/sidebarDragDrop.spec.ts @@ -154,7 +154,7 @@ test.describe("sidebar drag and drop", () => { // Get workspaceId from context for per-workspace layout key const workspaceId = ui.context.workspaceId; - // Set up a split layout via localStorage (simulating persistence) + // Seed Goal so its asynchronous restoration cannot race the tab-count check. // Layout key is per-workspace: "right-sidebar:layout:{workspaceId}" await page.evaluate( ({ wsId }) => { @@ -171,7 +171,7 @@ test.describe("sidebar drag and drop", () => { { type: "tabset", id: "tabset-1", - tabs: ["costs", "review"], + tabs: ["costs", "review", "goal"], activeTab: "costs", }, { @@ -201,9 +201,8 @@ test.describe("sidebar drag and drop", () => { const tablists = await sidebar.getByRole("tablist").all(); expect(tablists.length).toBe(2); - // Verify each tablist has expected tabs. The first tabset receives - // default-layout tabs injected by the migration (Stats, Review, Instructions). - await expect(tablists[0].getByRole("tab")).toHaveCount(3); + // The migration adds Instructions alongside the persisted Stats, Review, and Goal tabs. + await expect(tablists[0].getByRole("tab")).toHaveCount(4); await expect(tablists[1].getByRole("tab")).toHaveCount(1); // Stats (duplicate costs in split) }); diff --git a/tests/ipc/acp.configOptions.test.ts b/tests/ipc/acp.configOptions.test.ts index 7cc4a45e79a..dee42cc21c8 100644 --- a/tests/ipc/acp.configOptions.test.ts +++ b/tests/ipc/acp.configOptions.test.ts @@ -59,6 +59,7 @@ function createHarness( ): { client: ORPCClient; getWorkspaceState: () => WorkspaceState; + onAgentModeChanged: jest.Mock; updateModeCalls: Array<{ workspaceId: string; mode: "exec" | "plan"; @@ -70,7 +71,7 @@ function createHarness( aiSettings: WorkspaceAiSettings; }>; } { - let workspaceState: WorkspaceState = { + const workspaceState: WorkspaceState = { agentId: initial.agentId, aiSettings: { ...initial.aiSettings }, aiSettingsByAgent: { ...initial.aiSettingsByAgent }, @@ -111,16 +112,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: { @@ -130,16 +121,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 }; }, }, @@ -151,6 +132,7 @@ function createHarness( return { client, getWorkspaceState: () => workspaceState, + onAgentModeChanged: jest.fn(), updateModeCalls, updateAgentCalls, }; @@ -309,10 +291,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 () => { @@ -326,16 +309,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: { @@ -355,11 +364,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", }); @@ -370,8 +379,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", }); }); @@ -394,10 +403,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"); @@ -437,10 +448,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 9f34d791789..a8193be8f2f 100644 --- a/tests/ipc/acp.promptCorrelation.test.ts +++ b/tests/ipc/acp.promptCorrelation.test.ts @@ -566,6 +566,54 @@ function streamEnd( } describe("ACP prompt stream correlation", () => { + it.each(["unchanged", "loadSession", "resumeSession"] as const)( + "sends session-local picker settings after %s despite unchanged workspace metadata", + async (method) => { + 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 restored = + method === "unchanged" + ? undefined + : await harness.agent[method]({ + sessionId, + cwd: "/repo/acp-go-sdk", + mcpServers: [], + }); + + const { promptPromise, promptCorrelationId } = await startPromptTurn(harness, sessionId); + 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; + expect(harness.sendMessageCalls[0]?.options).toMatchObject({ + agentId: "plan", + model: "openai:gpt-5.2", + thinkingLevel: "high", + }); + if (restored) { + expect(restored.configOptions).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: "model", currentValue: "openai:gpt-5.2" }), + expect.objectContaining({ id: "thinkingLevel", currentValue: "high" }), + ]) + ); + } + } + ); + it("ignores unrelated stream-start/end pairs while waiting for this prompt turn", async () => { const harness = createHarness(); const { newSessionResponse, promptPromise, promptCorrelationId } =