Skip to content

Commit 017ef1b

Browse files
daniel-lxsdaniel-lxs
committed
refactor: flatten image generation settings structure
- Move openRouterImageApiKey and imageGenerationSelectedModel to top-level - Remove nested imageGenerationSettings object - Update all dependent components and tests - Handle secrets properly with GLOBAL_SECRET_KEYS - Simplify ContextProxy to handle flattened structure
1 parent 01458f1 commit 017ef1b

File tree

15 files changed

+192
-106
lines changed

15 files changed

+192
-106
lines changed

‎packages/types/src/global-settings.ts‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ export const globalSettingsSchema = z.object({
4242
customInstructions: z.string().optional(),
4343
taskHistory: z.array(historyItemSchema).optional(),
4444

45+
// Image generation settings (experimental) - flattened for simplicity
46+
imageGenerationSelectedModel: z.string().optional(),
47+
4548
condensingApiConfigId: z.string().optional(),
4649
customCondensingPrompt: z.string().optional(),
4750

@@ -200,11 +203,24 @@ export const SECRET_STATE_KEYS = [
200203
"featherlessApiKey",
201204
"ioIntelligenceApiKey",
202205
"vercelAiGatewayApiKey",
203-
] as const satisfies readonly (keyof ProviderSettings)[]
204-
export type SecretState = Pick<ProviderSettings, (typeof SECRET_STATE_KEYS)[number]>
206+
] as const
207+
208+
// Global secrets that are part of GlobalSettings (not ProviderSettings)
209+
export const GLOBAL_SECRET_KEYS = [
210+
"openRouterImageApiKey", // For image generation
211+
] as const
212+
213+
// Type for the actual secret storage keys
214+
type ProviderSecretKey = (typeof SECRET_STATE_KEYS)[number]
215+
type GlobalSecretKey = (typeof GLOBAL_SECRET_KEYS)[number]
216+
217+
// Type representing all secrets that can be stored
218+
export type SecretState = Pick<ProviderSettings, Extract<ProviderSecretKey, keyof ProviderSettings>> & {
219+
[K in GlobalSecretKey]?: string
220+
}
205221

206222
export const isSecretStateKey = (key: string): key is Keys<SecretState> =>
207-
SECRET_STATE_KEYS.includes(key as Keys<SecretState>)
223+
SECRET_STATE_KEYS.includes(key as ProviderSecretKey) || GLOBAL_SECRET_KEYS.includes(key as GlobalSecretKey)
208224

209225
/**
210226
* GlobalState
@@ -213,7 +229,7 @@ export const isSecretStateKey = (key: string): key is Keys<SecretState> =>
213229
export type GlobalState = Omit<RooCodeSettings, Keys<SecretState>>
214230

215231
export const GLOBAL_STATE_KEYS = [...GLOBAL_SETTINGS_KEYS, ...PROVIDER_SETTINGS_KEYS].filter(
216-
(key: Keys<RooCodeSettings>) => !SECRET_STATE_KEYS.includes(key as Keys<SecretState>),
232+
(key: Keys<RooCodeSettings>) => !isSecretStateKey(key),
217233
) as Keys<GlobalState>[]
218234

219235
export const isGlobalStateKey = (key: string): key is Keys<GlobalState> =>

‎packages/types/src/provider-settings.ts‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -142,13 +142,6 @@ const openRouterSchema = baseProviderSettingsSchema.extend({
142142
openRouterBaseUrl: z.string().optional(),
143143
openRouterSpecificProvider: z.string().optional(),
144144
openRouterUseMiddleOutTransform: z.boolean().optional(),
145-
// Image generation settings (experimental)
146-
openRouterImageGenerationSettings: z
147-
.object({
148-
openRouterApiKey: z.string().optional(),
149-
selectedModel: z.string().optional(),
150-
})
151-
.optional(),
152145
})
153146

154147
const bedrockSchema = apiModelIdProviderModelSchema.extend({

‎src/core/config/ContextProxy.ts‎

Lines changed: 57 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
GLOBAL_SETTINGS_KEYS,
77
SECRET_STATE_KEYS,
88
GLOBAL_STATE_KEYS,
9+
GLOBAL_SECRET_KEYS,
910
type ProviderSettings,
1011
type GlobalSettings,
1112
type SecretState,
@@ -61,13 +62,26 @@ export class ContextProxy {
6162
}
6263
}
6364

64-
const promises = SECRET_STATE_KEYS.map(async (key) => {
65-
try {
66-
this.secretCache[key] = await this.originalContext.secrets.get(key)
67-
} catch (error) {
68-
logger.error(`Error loading secret ${key}: ${error instanceof Error ? error.message : String(error)}`)
69-
}
70-
})
65+
const promises = [
66+
...SECRET_STATE_KEYS.map(async (key) => {
67+
try {
68+
this.secretCache[key] = await this.originalContext.secrets.get(key)
69+
} catch (error) {
70+
logger.error(
71+
`Error loading secret ${key}: ${error instanceof Error ? error.message : String(error)}`,
72+
)
73+
}
74+
}),
75+
...GLOBAL_SECRET_KEYS.map(async (key) => {
76+
try {
77+
this.secretCache[key] = await this.originalContext.secrets.get(key)
78+
} catch (error) {
79+
logger.error(
80+
`Error loading global secret ${key}: ${error instanceof Error ? error.message : String(error)}`,
81+
)
82+
}
83+
}),
84+
]
7185

7286
await Promise.all(promises)
7387

@@ -152,20 +166,34 @@ export class ContextProxy {
152166
* This is useful when you need to ensure the cache has the latest values
153167
*/
154168
async refreshSecrets(): Promise<void> {
155-
const promises = SECRET_STATE_KEYS.map(async (key) => {
156-
try {
157-
this.secretCache[key] = await this.originalContext.secrets.get(key)
158-
} catch (error) {
159-
logger.error(
160-
`Error refreshing secret ${key}: ${error instanceof Error ? error.message : String(error)}`,
161-
)
162-
}
163-
})
169+
const promises = [
170+
...SECRET_STATE_KEYS.map(async (key) => {
171+
try {
172+
this.secretCache[key] = await this.originalContext.secrets.get(key)
173+
} catch (error) {
174+
logger.error(
175+
`Error refreshing secret ${key}: ${error instanceof Error ? error.message : String(error)}`,
176+
)
177+
}
178+
}),
179+
...GLOBAL_SECRET_KEYS.map(async (key) => {
180+
try {
181+
this.secretCache[key] = await this.originalContext.secrets.get(key)
182+
} catch (error) {
183+
logger.error(
184+
`Error refreshing global secret ${key}: ${error instanceof Error ? error.message : String(error)}`,
185+
)
186+
}
187+
}),
188+
]
164189
await Promise.all(promises)
165190
}
166191

167192
private getAllSecretState(): SecretState {
168-
return Object.fromEntries(SECRET_STATE_KEYS.map((key) => [key, this.getSecret(key)]))
193+
return Object.fromEntries([
194+
...SECRET_STATE_KEYS.map((key) => [key, this.getSecret(key as SecretStateKey)]),
195+
...GLOBAL_SECRET_KEYS.map((key) => [key, this.getSecret(key as SecretStateKey)]),
196+
])
169197
}
170198

171199
/**
@@ -232,18 +260,24 @@ export class ContextProxy {
232260
* RooCodeSettings
233261
*/
234262

235-
public setValue<K extends RooCodeSettingsKey>(key: K, value: RooCodeSettings[K]) {
236-
return isSecretStateKey(key) ? this.storeSecret(key, value as string) : this.updateGlobalState(key, value)
263+
public async setValue<K extends RooCodeSettingsKey>(key: K, value: RooCodeSettings[K]) {
264+
return isSecretStateKey(key)
265+
? this.storeSecret(key as SecretStateKey, value as string)
266+
: this.updateGlobalState(key as GlobalStateKey, value)
237267
}
238268

239269
public getValue<K extends RooCodeSettingsKey>(key: K): RooCodeSettings[K] {
240270
return isSecretStateKey(key)
241-
? (this.getSecret(key) as RooCodeSettings[K])
242-
: (this.getGlobalState(key) as RooCodeSettings[K])
271+
? (this.getSecret(key as SecretStateKey) as RooCodeSettings[K])
272+
: (this.getGlobalState(key as GlobalStateKey) as RooCodeSettings[K])
243273
}
244274

245275
public getValues(): RooCodeSettings {
246-
return { ...this.getAllGlobalState(), ...this.getAllSecretState() }
276+
const globalState = this.getAllGlobalState()
277+
const secretState = this.getAllSecretState()
278+
279+
// Simply merge all states - no nested secrets to handle
280+
return { ...globalState, ...secretState }
247281
}
248282

249283
public async setValues(values: RooCodeSettings) {
@@ -285,6 +319,7 @@ export class ContextProxy {
285319
await Promise.all([
286320
...GLOBAL_STATE_KEYS.map((key) => this.originalContext.globalState.update(key, undefined)),
287321
...SECRET_STATE_KEYS.map((key) => this.originalContext.secrets.delete(key)),
322+
...GLOBAL_SECRET_KEYS.map((key) => this.originalContext.secrets.delete(key)),
288323
])
289324

290325
await this.initialize()

‎src/core/tools/generateImageTool.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -129,10 +129,8 @@ export async function generateImageTool(
129129
// Check if file is write-protected
130130
const isWriteProtected = cline.rooProtectedController?.isWriteProtected(relPath) || false
131131

132-
// Get OpenRouter API key from experimental settings ONLY (no fallback to profile)
133-
const apiConfiguration = state?.apiConfiguration
134-
const imageGenerationSettings = apiConfiguration?.openRouterImageGenerationSettings
135-
const openRouterApiKey = imageGenerationSettings?.openRouterApiKey
132+
// Get OpenRouter API key from global settings (experimental image generation)
133+
const openRouterApiKey = state?.openRouterImageApiKey
136134

137135
if (!openRouterApiKey) {
138136
await cline.say(
@@ -148,7 +146,7 @@ export async function generateImageTool(
148146
}
149147

150148
// Get selected model from settings or use default
151-
const selectedModel = imageGenerationSettings?.selectedModel || IMAGE_GENERATION_MODELS[0]
149+
const selectedModel = state?.imageGenerationSelectedModel || IMAGE_GENERATION_MODELS[0]
152150

153151
// Determine if the path is outside the workspace
154152
const fullPath = path.resolve(cline.cwd, removeClosingTag("path", relPath))

‎src/core/webview/ClineProvider.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1762,6 +1762,8 @@ export class ClineProvider
17621762
maxDiagnosticMessages,
17631763
includeTaskHistoryInEnhance,
17641764
remoteControlEnabled,
1765+
openRouterImageApiKey,
1766+
imageGenerationSelectedModel,
17651767
} = await this.getState()
17661768

17671769
const telemetryKey = process.env.POSTHOG_API_KEY
@@ -1893,6 +1895,8 @@ export class ClineProvider
18931895
maxDiagnosticMessages: maxDiagnosticMessages ?? 50,
18941896
includeTaskHistoryInEnhance: includeTaskHistoryInEnhance ?? true,
18951897
remoteControlEnabled,
1898+
openRouterImageApiKey,
1899+
imageGenerationSelectedModel,
18961900
}
18971901
}
18981902

@@ -2092,6 +2096,9 @@ export class ClineProvider
20922096
return false
20932097
}
20942098
})(),
2099+
// Add image generation settings
2100+
openRouterImageApiKey: (stateValues as any).openRouterImageApiKey,
2101+
imageGenerationSelectedModel: stateValues.imageGenerationSelectedModel,
20952102
}
20962103
}
20972104

‎src/core/webview/__tests__/ClineProvider.spec.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,8 @@ describe("ClineProvider", () => {
546546
profileThresholds: {},
547547
hasOpenedModeSelector: false,
548548
diagnosticsEnabled: true,
549+
openRouterImageApiKey: undefined,
550+
imageGenerationSelectedModel: undefined,
549551
}
550552

551553
const message: ExtensionMessage = {

‎src/core/webview/webviewMessageHandler.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1313,6 +1313,14 @@ export const webviewMessageHandler = async (
13131313
await updateGlobalState("language", message.text as Language)
13141314
await provider.postStateToWebview()
13151315
break
1316+
case "openRouterImageApiKey":
1317+
await provider.contextProxy.setValue("openRouterImageApiKey", message.text)
1318+
await provider.postStateToWebview()
1319+
break
1320+
case "imageGenerationSelectedModel":
1321+
await provider.contextProxy.setValue("imageGenerationSelectedModel", message.text)
1322+
await provider.postStateToWebview()
1323+
break
13161324
case "showRooIgnoredFiles":
13171325
await updateGlobalState("showRooIgnoredFiles", message.bool ?? false)
13181326
await provider.postStateToWebview()

‎src/shared/ExtensionMessage.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,7 @@ export type ExtensionState = Pick<
273273
| "includeDiagnosticMessages"
274274
| "maxDiagnosticMessages"
275275
| "remoteControlEnabled"
276+
| "imageGenerationSelectedModel"
276277
> & {
277278
version: string
278279
clineMessages: ClineMessage[]
@@ -326,6 +327,7 @@ export type ExtensionState = Pick<
326327
marketplaceInstalledMetadata?: { project: Record<string, any>; global: Record<string, any> }
327328
profileThresholds: Record<string, number>
328329
hasOpenedModeSelector: boolean
330+
openRouterImageApiKey?: string
329331
}
330332

331333
export interface ClineSayTool {

‎src/shared/WebviewMessage.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,9 @@ export interface WebviewMessage {
212212
| "createCommand"
213213
| "insertTextIntoTextarea"
214214
| "showMdmAuthRequiredNotification"
215+
| "imageGenerationSettings"
216+
| "openRouterImageApiKey"
217+
| "imageGenerationSelectedModel"
215218
text?: string
216219
editedMessageContent?: string
217220
tab?: "settings" | "history" | "mcp" | "modes" | "chat" | "marketplace" | "account"
@@ -248,6 +251,7 @@ export interface WebviewMessage {
248251
messageTs?: number
249252
historyPreviewCollapsed?: boolean
250253
filters?: { type?: string; search?: string; tags?: string[] }
254+
settings?: any
251255
url?: string // For openExternal
252256
mpItem?: MarketplaceItem
253257
mpInstallOptions?: InstallMarketplaceItemOptions

‎src/shared/checkExistApiConfig.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { SECRET_STATE_KEYS, ProviderSettings } from "@roo-code/types"
1+
import { SECRET_STATE_KEYS, GLOBAL_SECRET_KEYS, ProviderSettings } from "@roo-code/types"
22

33
export function checkExistKey(config: ProviderSettings | undefined) {
44
if (!config) {
@@ -14,7 +14,9 @@ export function checkExistKey(config: ProviderSettings | undefined) {
1414
}
1515

1616
// Check all secret keys from the centralized SECRET_STATE_KEYS array.
17-
const hasSecretKey = SECRET_STATE_KEYS.some((key) => config[key] !== undefined)
17+
// Filter out keys that are not part of ProviderSettings (global secrets are stored separately)
18+
const providerSecretKeys = SECRET_STATE_KEYS.filter((key) => !GLOBAL_SECRET_KEYS.includes(key as any))
19+
const hasSecretKey = providerSecretKeys.some((key) => config[key as keyof ProviderSettings] !== undefined)
1820

1921
// Check additional non-secret configuration properties
2022
const hasOtherConfig = [

0 commit comments

Comments
 (0)