From a96fe08b1810c74e8b9a053de7e39c8971aa7686 Mon Sep 17 00:00:00 2001 From: Daniel <57051444+daniel-lxs@users.noreply.github.com> Date: Fri, 29 Aug 2025 17:35:24 -0500 Subject: [PATCH] refactor: flatten image generation settings structure (#7536) --- packages/types/src/provider-settings.ts | 7 --- .../tools/__tests__/generateImageTool.test.ts | 8 +-- src/core/tools/generateImageTool.ts | 8 ++- src/core/webview/ClineProvider.ts | 7 +++ .../webview/__tests__/ClineProvider.spec.ts | 2 + src/core/webview/webviewMessageHandler.ts | 8 +++ src/shared/ExtensionMessage.ts | 2 + src/shared/WebviewMessage.ts | 4 ++ .../settings/ExperimentalSettings.tsx | 20 +++++-- .../settings/ImageGenerationSettings.tsx | 52 +++++++----------- .../src/components/settings/SettingsView.tsx | 35 ++++++++++-- .../ImageGenerationSettings.spec.tsx | 53 +++++++++---------- 12 files changed, 120 insertions(+), 86 deletions(-) diff --git a/packages/types/src/provider-settings.ts b/packages/types/src/provider-settings.ts index 878c0c1127..4dfeacbf07 100644 --- a/packages/types/src/provider-settings.ts +++ b/packages/types/src/provider-settings.ts @@ -143,13 +143,6 @@ const openRouterSchema = baseProviderSettingsSchema.extend({ openRouterBaseUrl: z.string().optional(), openRouterSpecificProvider: z.string().optional(), openRouterUseMiddleOutTransform: z.boolean().optional(), - // Image generation settings (experimental) - openRouterImageGenerationSettings: z - .object({ - openRouterApiKey: z.string().optional(), - selectedModel: z.string().optional(), - }) - .optional(), }) const bedrockSchema = apiModelIdProviderModelSchema.extend({ diff --git a/src/core/tools/__tests__/generateImageTool.test.ts b/src/core/tools/__tests__/generateImageTool.test.ts index ac7e122841..0a12bebbe2 100644 --- a/src/core/tools/__tests__/generateImageTool.test.ts +++ b/src/core/tools/__tests__/generateImageTool.test.ts @@ -46,12 +46,8 @@ describe("generateImageTool", () => { experiments: { [EXPERIMENT_IDS.IMAGE_GENERATION]: true, }, - apiConfiguration: { - openRouterImageGenerationSettings: { - openRouterApiKey: "test-api-key", - selectedModel: "google/gemini-2.5-flash-image-preview", - }, - }, + openRouterImageApiKey: "test-api-key", + openRouterImageGenerationSelectedModel: "google/gemini-2.5-flash-image-preview", }), }), }, diff --git a/src/core/tools/generateImageTool.ts b/src/core/tools/generateImageTool.ts index 4bb67d629c..775637f34b 100644 --- a/src/core/tools/generateImageTool.ts +++ b/src/core/tools/generateImageTool.ts @@ -129,10 +129,8 @@ export async function generateImageTool( // Check if file is write-protected const isWriteProtected = cline.rooProtectedController?.isWriteProtected(relPath) || false - // Get OpenRouter API key from experimental settings ONLY (no fallback to profile) - const apiConfiguration = state?.apiConfiguration - const imageGenerationSettings = apiConfiguration?.openRouterImageGenerationSettings - const openRouterApiKey = imageGenerationSettings?.openRouterApiKey + // Get OpenRouter API key from global settings (experimental image generation) + const openRouterApiKey = state?.openRouterImageApiKey if (!openRouterApiKey) { await cline.say( @@ -148,7 +146,7 @@ export async function generateImageTool( } // Get selected model from settings or use default - const selectedModel = imageGenerationSettings?.selectedModel || IMAGE_GENERATION_MODELS[0] + const selectedModel = state?.openRouterImageGenerationSelectedModel || IMAGE_GENERATION_MODELS[0] // Determine if the path is outside the workspace const fullPath = path.resolve(cline.cwd, removeClosingTag("path", relPath)) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 003b4fe84c..1e9aa28758 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1820,6 +1820,8 @@ export class ClineProvider maxDiagnosticMessages, includeTaskHistoryInEnhance, remoteControlEnabled, + openRouterImageApiKey, + openRouterImageGenerationSelectedModel, } = await this.getState() const telemetryKey = process.env.POSTHOG_API_KEY @@ -1952,6 +1954,8 @@ export class ClineProvider includeTaskHistoryInEnhance: includeTaskHistoryInEnhance ?? true, remoteControlEnabled, filesChangedEnabled: this.getGlobalState("filesChangedEnabled") ?? true, + openRouterImageApiKey, + openRouterImageGenerationSelectedModel, } } @@ -2151,6 +2155,9 @@ export class ClineProvider return false } })(), + // Add image generation settings + openRouterImageApiKey: stateValues.openRouterImageApiKey, + openRouterImageGenerationSelectedModel: stateValues.openRouterImageGenerationSelectedModel, } } diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index ca2bb145f9..2d106782b6 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -584,6 +584,8 @@ describe("ClineProvider", () => { openRouterImageApiKey: undefined, openRouterImageGenerationSelectedModel: undefined, filesChangedEnabled: true, + openRouterImageApiKey: undefined, + openRouterImageGenerationSelectedModel: undefined, } const message: ExtensionMessage = { diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 8b024f2223..2dda4c32c0 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -1314,6 +1314,14 @@ export const webviewMessageHandler = async ( await updateGlobalState("language", message.text as Language) await provider.postStateToWebview() break + case "openRouterImageApiKey": + await provider.contextProxy.setValue("openRouterImageApiKey", message.text) + await provider.postStateToWebview() + break + case "openRouterImageGenerationSelectedModel": + await provider.contextProxy.setValue("openRouterImageGenerationSelectedModel", message.text) + await provider.postStateToWebview() + break case "showRooIgnoredFiles": await updateGlobalState("showRooIgnoredFiles", message.bool ?? false) await provider.postStateToWebview() diff --git a/src/shared/ExtensionMessage.ts b/src/shared/ExtensionMessage.ts index b39ebdf91b..8a7bc9bb62 100644 --- a/src/shared/ExtensionMessage.ts +++ b/src/shared/ExtensionMessage.ts @@ -286,6 +286,7 @@ export type ExtensionState = Pick< | "includeDiagnosticMessages" | "maxDiagnosticMessages" | "remoteControlEnabled" + | "openRouterImageGenerationSelectedModel" > & { version: string clineMessages: ClineMessage[] @@ -340,6 +341,7 @@ export type ExtensionState = Pick< profileThresholds: Record hasOpenedModeSelector: boolean filesChangedEnabled: boolean + openRouterImageApiKey?: string } export interface ClineSayTool { diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index a66fa8c79c..07cd8c79b6 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -218,6 +218,9 @@ export interface WebviewMessage { | "filesChangedEnabled" | "filesChangedRequest" | "filesChangedBaselineUpdate" + | "imageGenerationSettings" + | "openRouterImageApiKey" + | "openRouterImageGenerationSelectedModel" text?: string editedMessageContent?: string tab?: "settings" | "history" | "mcp" | "modes" | "chat" | "marketplace" | "account" @@ -254,6 +257,7 @@ export interface WebviewMessage { messageTs?: number historyPreviewCollapsed?: boolean filters?: { type?: string; search?: string; tags?: string[] } + settings?: any url?: string // For openExternal mpItem?: MarketplaceItem mpInstallOptions?: InstallMarketplaceItemOptions diff --git a/webview-ui/src/components/settings/ExperimentalSettings.tsx b/webview-ui/src/components/settings/ExperimentalSettings.tsx index 7df649354d..b9ea7d5737 100644 --- a/webview-ui/src/components/settings/ExperimentalSettings.tsx +++ b/webview-ui/src/components/settings/ExperimentalSettings.tsx @@ -23,6 +23,10 @@ type ExperimentalSettingsProps = HTMLAttributes & { setCachedStateField?: SetCachedStateField<"filesChangedEnabled"> apiConfiguration?: any setApiConfigurationField?: any + openRouterImageApiKey?: string + openRouterImageGenerationSelectedModel?: string + setOpenRouterImageApiKey?: (apiKey: string) => void + setImageGenerationSelectedModel?: (model: string) => void } export const ExperimentalSettings = ({ @@ -32,6 +36,10 @@ export const ExperimentalSettings = ({ setCachedStateField, apiConfiguration, setApiConfigurationField, + openRouterImageApiKey, + openRouterImageGenerationSelectedModel, + setOpenRouterImageApiKey, + setImageGenerationSelectedModel, className, ...props }: ExperimentalSettingsProps) => { @@ -80,7 +88,11 @@ export const ExperimentalSettings = ({ /> ) } - if (config[0] === "IMAGE_GENERATION" && apiConfiguration && setApiConfigurationField) { + if ( + config[0] === "IMAGE_GENERATION" && + setOpenRouterImageApiKey && + setImageGenerationSelectedModel + ) { return ( setExperimentEnabled(EXPERIMENT_IDS.IMAGE_GENERATION, enabled) } - apiConfiguration={apiConfiguration} - setApiConfigurationField={setApiConfigurationField} + openRouterImageApiKey={openRouterImageApiKey} + openRouterImageGenerationSelectedModel={openRouterImageGenerationSelectedModel} + setOpenRouterImageApiKey={setOpenRouterImageApiKey} + setImageGenerationSelectedModel={setImageGenerationSelectedModel} /> ) } diff --git a/webview-ui/src/components/settings/ImageGenerationSettings.tsx b/webview-ui/src/components/settings/ImageGenerationSettings.tsx index 800e981fe9..c31f31e316 100644 --- a/webview-ui/src/components/settings/ImageGenerationSettings.tsx +++ b/webview-ui/src/components/settings/ImageGenerationSettings.tsx @@ -1,17 +1,14 @@ import React, { useState, useEffect } from "react" import { VSCodeCheckbox, VSCodeTextField, VSCodeDropdown, VSCodeOption } from "@vscode/webview-ui-toolkit/react" import { useAppTranslation } from "@/i18n/TranslationContext" -import type { ProviderSettings } from "@roo-code/types" interface ImageGenerationSettingsProps { enabled: boolean onChange: (enabled: boolean) => void - apiConfiguration: ProviderSettings - setApiConfigurationField: ( - field: K, - value: ProviderSettings[K], - isUserAction?: boolean, - ) => void + openRouterImageApiKey?: string + openRouterImageGenerationSelectedModel?: string + setOpenRouterImageApiKey: (apiKey: string) => void + setImageGenerationSelectedModel: (model: string) => void } // Hardcoded list of image generation models @@ -24,43 +21,34 @@ const IMAGE_GENERATION_MODELS = [ export const ImageGenerationSettings = ({ enabled, onChange, - apiConfiguration, - setApiConfigurationField, + openRouterImageApiKey, + openRouterImageGenerationSelectedModel, + setOpenRouterImageApiKey, + setImageGenerationSelectedModel, }: ImageGenerationSettingsProps) => { const { t } = useAppTranslation() - // Get image generation settings from apiConfiguration - const imageGenerationSettings = apiConfiguration?.openRouterImageGenerationSettings || {} - const [openRouterApiKey, setOpenRouterApiKey] = useState(imageGenerationSettings.openRouterApiKey || "") + const [apiKey, setApiKey] = useState(openRouterImageApiKey || "") const [selectedModel, setSelectedModel] = useState( - imageGenerationSettings.selectedModel || IMAGE_GENERATION_MODELS[0].value, + openRouterImageGenerationSelectedModel || IMAGE_GENERATION_MODELS[0].value, ) - // Update local state when apiConfiguration changes (e.g., when switching profiles) + // Update local state when props change (e.g., when switching profiles) useEffect(() => { - setOpenRouterApiKey(imageGenerationSettings.openRouterApiKey || "") - setSelectedModel(imageGenerationSettings.selectedModel || IMAGE_GENERATION_MODELS[0].value) - }, [imageGenerationSettings.openRouterApiKey, imageGenerationSettings.selectedModel]) - - // Helper function to update settings - const updateSettings = (newApiKey: string, newModel: string) => { - const newSettings = { - openRouterApiKey: newApiKey, - selectedModel: newModel, - } - setApiConfigurationField("openRouterImageGenerationSettings", newSettings, true) - } + setApiKey(openRouterImageApiKey || "") + setSelectedModel(openRouterImageGenerationSelectedModel || IMAGE_GENERATION_MODELS[0].value) + }, [openRouterImageApiKey, openRouterImageGenerationSelectedModel]) // Handle API key changes const handleApiKeyChange = (value: string) => { - setOpenRouterApiKey(value) - updateSettings(value, selectedModel) + setApiKey(value) + setOpenRouterImageApiKey(value) } // Handle model selection changes const handleModelChange = (value: string) => { setSelectedModel(value) - updateSettings(openRouterApiKey, value) + setImageGenerationSelectedModel(value) } return ( @@ -84,7 +72,7 @@ export const ImageGenerationSettings = ({ {t("settings:experimental.IMAGE_GENERATION.openRouterApiKeyLabel")} handleApiKeyChange(e.target.value)} placeholder={t("settings:experimental.IMAGE_GENERATION.openRouterApiKeyPlaceholder")} className="w-full" @@ -123,13 +111,13 @@ export const ImageGenerationSettings = ({ {/* Status Message */} - {enabled && !openRouterApiKey && ( + {enabled && !apiKey && (
{t("settings:experimental.IMAGE_GENERATION.warningMissingKey")}
)} - {enabled && openRouterApiKey && ( + {enabled && apiKey && (
{t("settings:experimental.IMAGE_GENERATION.successConfigured")}
diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx index 667ddb310f..6aae1a6b0b 100644 --- a/webview-ui/src/components/settings/SettingsView.tsx +++ b/webview-ui/src/components/settings/SettingsView.tsx @@ -183,6 +183,8 @@ const SettingsView = forwardRef(({ onDone, t maxDiagnosticMessages, includeTaskHistoryInEnhance, filesChangedEnabled, + openRouterImageApiKey, + openRouterImageGenerationSelectedModel, } = cachedState const apiConfiguration = useMemo(() => cachedState.apiConfiguration ?? {}, [cachedState.apiConfiguration]) @@ -262,6 +264,20 @@ const SettingsView = forwardRef(({ onDone, t }) }, []) + const setOpenRouterImageApiKey = useCallback((apiKey: string) => { + setCachedState((prevState) => { + setChangeDetected(true) + return { ...prevState, openRouterImageApiKey: apiKey } + }) + }, []) + + const setImageGenerationSelectedModel = useCallback((model: string) => { + setCachedState((prevState) => { + setChangeDetected(true) + return { ...prevState, openRouterImageGenerationSelectedModel: model } + }) + }, []) + const setCustomSupportPromptsField = useCallback((prompts: Record) => { setCachedState((prevState) => { if (JSON.stringify(prevState.customSupportPrompts) === JSON.stringify(prompts)) { @@ -346,6 +362,11 @@ const SettingsView = forwardRef(({ onDone, t vscode.postMessage({ type: "upsertApiConfiguration", text: currentApiConfigName, apiConfiguration }) vscode.postMessage({ type: "telemetrySetting", text: telemetrySetting }) vscode.postMessage({ type: "profileThresholds", values: profileThresholds }) + vscode.postMessage({ type: "openRouterImageApiKey", text: openRouterImageApiKey }) + vscode.postMessage({ + type: "openRouterImageGenerationSelectedModel", + text: openRouterImageGenerationSelectedModel, + }) setChangeDetected(false) } } @@ -724,10 +745,16 @@ const SettingsView = forwardRef(({ onDone, t } - apiConfiguration={apiConfiguration} - setApiConfigurationField={setApiConfigurationField} + filesChangedEnabled={filesChangedEnabled} + setCachedStateField={setCachedStateField as SetCachedStateField<"filesChangedEnabled">} + apiConfiguration={apiConfiguration} + setApiConfigurationField={setApiConfigurationField} + openRouterImageApiKey={openRouterImageApiKey as string | undefined} + openRouterImageGenerationSelectedModel={ + openRouterImageGenerationSelectedModel as string | undefined + } + setOpenRouterImageApiKey={setOpenRouterImageApiKey} + setImageGenerationSelectedModel={setImageGenerationSelectedModel} /> )} diff --git a/webview-ui/src/components/settings/__tests__/ImageGenerationSettings.spec.tsx b/webview-ui/src/components/settings/__tests__/ImageGenerationSettings.spec.tsx index 2d69879772..cadd8f83e0 100644 --- a/webview-ui/src/components/settings/__tests__/ImageGenerationSettings.spec.tsx +++ b/webview-ui/src/components/settings/__tests__/ImageGenerationSettings.spec.tsx @@ -1,7 +1,5 @@ import { render, fireEvent } from "@testing-library/react" -import type { ProviderSettings } from "@roo-code/types" - import { ImageGenerationSettings } from "../ImageGenerationSettings" // Mock the translation context @@ -12,14 +10,17 @@ vi.mock("@/i18n/TranslationContext", () => ({ })) describe("ImageGenerationSettings", () => { - const mockSetApiConfigurationField = vi.fn() + const mockSetOpenRouterImageApiKey = vi.fn() + const mockSetImageGenerationSelectedModel = vi.fn() const mockOnChange = vi.fn() const defaultProps = { enabled: false, onChange: mockOnChange, - apiConfiguration: {} as ProviderSettings, - setApiConfigurationField: mockSetApiConfigurationField, + openRouterImageApiKey: undefined, + openRouterImageGenerationSelectedModel: undefined, + setOpenRouterImageApiKey: mockSetOpenRouterImageApiKey, + setImageGenerationSelectedModel: mockSetImageGenerationSelectedModel, } beforeEach(() => { @@ -27,30 +28,31 @@ describe("ImageGenerationSettings", () => { }) describe("Initial Mount Behavior", () => { - it("should not call setApiConfigurationField on initial mount with empty configuration", () => { + it("should not call setter functions on initial mount with empty configuration", () => { render() - // Should NOT call setApiConfigurationField on initial mount to prevent dirty state - expect(mockSetApiConfigurationField).not.toHaveBeenCalled() + // Should NOT call setter functions on initial mount to prevent dirty state + expect(mockSetOpenRouterImageApiKey).not.toHaveBeenCalled() + expect(mockSetImageGenerationSelectedModel).not.toHaveBeenCalled() }) - it("should not call setApiConfigurationField on initial mount with existing configuration", () => { - const apiConfiguration = { - openRouterImageGenerationSettings: { - openRouterApiKey: "existing-key", - selectedModel: "google/gemini-2.5-flash-image-preview:free", - }, - } as ProviderSettings + it("should not call setter functions on initial mount with existing configuration", () => { + render( + , + ) - render() - - // Should NOT call setApiConfigurationField on initial mount to prevent dirty state - expect(mockSetApiConfigurationField).not.toHaveBeenCalled() + // Should NOT call setter functions on initial mount to prevent dirty state + expect(mockSetOpenRouterImageApiKey).not.toHaveBeenCalled() + expect(mockSetImageGenerationSelectedModel).not.toHaveBeenCalled() }) }) describe("User Interaction Behavior", () => { - it("should call setApiConfigurationField when user changes API key", async () => { + it("should call setimageGenerationSettings when user changes API key", async () => { const { getByPlaceholderText } = render() const apiKeyInput = getByPlaceholderText( @@ -60,15 +62,8 @@ describe("ImageGenerationSettings", () => { // Simulate user typing fireEvent.input(apiKeyInput, { target: { value: "new-api-key" } }) - // Should call setApiConfigurationField with isUserAction=true - expect(mockSetApiConfigurationField).toHaveBeenCalledWith( - "openRouterImageGenerationSettings", - { - openRouterApiKey: "new-api-key", - selectedModel: "google/gemini-2.5-flash-image-preview", - }, - true, // This should be true for user actions - ) + // Should call setimageGenerationSettings + expect(defaultProps.setOpenRouterImageApiKey).toHaveBeenCalledWith("new-api-key") }) // Note: Testing VSCode dropdown components is complex due to their custom nature