Skip to content

Commit 7feb2df

Browse files
committed
🤖 fix: prevent model selection race condition in creation mode
There were two issues causing the wrong model to be used when creating workspaces: 1. **Stale React state**: The `handleSend` callback in `useCreationWorkspace` captured `sendMessageOptions` from `useSendMessageOptions`, but that hook's state updates were delayed by `requestAnimationFrame` batching in `usePersistedState`. If the user selected a model and clicked send before the next animation frame, the old model value would be used. 2. **Effect overwriting selection**: The useEffect that initialized the model to the default when entering creation mode could re-run and overwrite the user's selection if `defaultModel` changed for any reason. **Fixes:** - Read send options fresh from localStorage at send time using `getSendOptionsFromStorage` instead of relying on potentially-stale React state - Track initialization state with a ref so the model is only set once per entry into creation mode, not on every `defaultModel` change Also added `mode` field to `getSendOptionsFromStorage` return value for parity with `useSendMessageOptions`. _Generated with `mux`_
1 parent d7560e1 commit 7feb2df

File tree

4 files changed

+31
-22
lines changed

4 files changed

+31
-22
lines changed

src/browser/components/ChatInput/index.tsx

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -182,12 +182,21 @@ export const ChatInput: React.FC<ChatInputProps> = (props) => {
182182
[storageKeys.modelKey, addModel]
183183
);
184184

185-
// When entering creation mode (or when the default model changes), reset the
186-
// project-scoped model to the explicit default so manual picks don't bleed
187-
// into subsequent creation flows.
185+
// When entering creation mode, initialize the project-scoped model to the
186+
// default so previous manual picks don't bleed into new creation flows.
187+
// Only runs once per creation session (not when defaultModel changes, which
188+
// would clobber the user's intentional model selection).
189+
const creationModelInitialized = useRef<string | null>(null);
188190
useEffect(() => {
189191
if (variant === "creation" && defaultModel) {
190-
updatePersistedState(storageKeys.modelKey, defaultModel);
192+
// Only initialize once per project scope
193+
if (creationModelInitialized.current !== storageKeys.modelKey) {
194+
creationModelInitialized.current = storageKeys.modelKey;
195+
updatePersistedState(storageKeys.modelKey, defaultModel);
196+
}
197+
} else if (variant !== "creation") {
198+
// Reset when leaving creation mode so re-entering triggers initialization
199+
creationModelInitialized.current = null;
191200
}
192201
}, [variant, defaultModel, storageKeys.modelKey]);
193202

src/browser/components/ChatInput/useCreationWorkspace.test.tsx

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@ import type { DraftWorkspaceSettings } from "@/browser/hooks/useDraftWorkspaceSe
22
import {
33
getInputKey,
44
getModeKey,
5+
getModelKey,
56
getPendingScopeId,
67
getProjectScopeId,
78
getThinkingLevelKey,
89
} from "@/common/constants/storage";
910
import type { SendMessageError } from "@/common/types/errors";
10-
import type { BranchListResult, IPCApi, SendMessageOptions } from "@/common/types/ipc";
11+
import type { BranchListResult, IPCApi } from "@/common/types/ipc";
1112
import type { RuntimeMode } from "@/common/types/runtime";
1213
import type { FrontendWorkspaceMetadata } from "@/common/types/workspace";
1314
import { act, cleanup, render, waitFor } from "@testing-library/react";
@@ -56,14 +57,8 @@ void mock.module("@/browser/hooks/useDraftWorkspaceSettings", () => ({
5657
useDraftWorkspaceSettings: useDraftWorkspaceSettingsMock,
5758
}));
5859

59-
let currentSendOptions: SendMessageOptions;
60-
const useSendMessageOptionsMock = mock(() => currentSendOptions);
61-
6260
type WorkspaceSendMessage = IPCApi["workspace"]["sendMessage"];
6361
type WorkspaceSendMessageParams = Parameters<WorkspaceSendMessage>;
64-
void mock.module("@/browser/hooks/useSendMessageOptions", () => ({
65-
useSendMessageOptions: useSendMessageOptionsMock,
66-
}));
6762

6863
const TEST_PROJECT_PATH = "/projects/demo";
6964
const TEST_WORKSPACE_ID = "ws-created";
@@ -86,11 +81,6 @@ describe("useCreationWorkspace", () => {
8681
updatePersistedStateCalls.length = 0;
8782
draftSettingsInvocations = [];
8883
draftSettingsState = createDraftSettingsHarness();
89-
currentSendOptions = {
90-
model: "gpt-4",
91-
thinkingLevel: "medium",
92-
mode: "exec",
93-
} satisfies SendMessageOptions;
9484
});
9585

9686
afterEach(() => {
@@ -180,6 +170,8 @@ describe("useCreationWorkspace", () => {
180170

181171
persistedPreferences[getModeKey(getProjectScopeId(TEST_PROJECT_PATH))] = "plan";
182172
persistedPreferences[getThinkingLevelKey(getProjectScopeId(TEST_PROJECT_PATH))] = "high";
173+
// Set model preference for the project scope (read by getSendOptionsFromStorage)
174+
persistedPreferences[getModelKey(getProjectScopeId(TEST_PROJECT_PATH))] = "gpt-4";
183175

184176
draftSettingsState = createDraftSettingsHarness({
185177
runtimeMode: "ssh",
@@ -207,8 +199,10 @@ describe("useCreationWorkspace", () => {
207199
expect(options?.projectPath).toBe(TEST_PROJECT_PATH);
208200
expect(options?.trunkBranch).toBe("dev");
209201
expect(options?.model).toBe("gpt-4");
210-
expect(options?.mode).toBe("exec");
211-
expect(options?.thinkingLevel).toBe("medium");
202+
// Mode was set to "plan" in persistedPreferences, so that's what we expect
203+
expect(options?.mode).toBe("plan");
204+
// thinkingLevel was set to "high" in persistedPreferences
205+
expect(options?.thinkingLevel).toBe("high");
212206
expect(options?.runtimeConfig).toEqual({
213207
type: "ssh",
214208
host: "example.com",

src/browser/components/ChatInput/useCreationWorkspace.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import type { ThinkingLevel } from "@/common/types/thinking";
66
import { parseRuntimeString } from "@/browser/utils/chatCommands";
77
import { useDraftWorkspaceSettings } from "@/browser/hooks/useDraftWorkspaceSettings";
88
import { readPersistedState, updatePersistedState } from "@/browser/hooks/usePersistedState";
9-
import { useSendMessageOptions } from "@/browser/hooks/useSendMessageOptions";
9+
import { getSendOptionsFromStorage } from "@/browser/utils/messages/sendOptions";
1010
import {
1111
getInputKey,
1212
getModeKey,
@@ -72,8 +72,8 @@ export function useCreationWorkspace({
7272
const { settings, setRuntimeOptions, setTrunkBranch, getRuntimeString } =
7373
useDraftWorkspaceSettings(projectPath, branches, recommendedTrunk);
7474

75-
// Get send options from shared hook (uses project-scoped storage key)
76-
const sendMessageOptions = useSendMessageOptions(getProjectScopeId(projectPath));
75+
// Project scope ID for reading send options at send time
76+
const projectScopeId = getProjectScopeId(projectPath);
7777

7878
// Load branches on mount
7979
useEffect(() => {
@@ -108,6 +108,11 @@ export function useCreationWorkspace({
108108
? parseRuntimeString(runtimeString, "")
109109
: undefined;
110110

111+
// Read send options fresh from localStorage at send time to avoid
112+
// race conditions with React state updates (requestAnimationFrame batching
113+
// in usePersistedState can delay state updates after model selection)
114+
const sendMessageOptions = getSendOptionsFromStorage(projectScopeId);
115+
111116
// Send message with runtime config and creation-specific params
112117
const result = await window.api.workspace.sendMessage(null, message, {
113118
...sendMessageOptions,
@@ -158,9 +163,9 @@ export function useCreationWorkspace({
158163
[
159164
isSending,
160165
projectPath,
166+
projectScopeId,
161167
onWorkspaceCreated,
162168
getRuntimeString,
163-
sendMessageOptions,
164169
settings.trunkBranch,
165170
]
166171
);

src/browser/utils/messages/sendOptions.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ export function getSendOptionsFromStorage(workspaceId: string): SendMessageOptio
6060

6161
return {
6262
model,
63+
mode: mode === "exec" || mode === "plan" ? mode : "exec", // Only pass exec/plan to backend
6364
thinkingLevel: effectiveThinkingLevel,
6465
toolPolicy: modeToToolPolicy(mode),
6566
additionalSystemInstructions,

0 commit comments

Comments
 (0)