Skip to content

Commit 8f8d323

Browse files
committed
fix: address PR feedback - add migration logic and fix tests
- Added openRouterImageApiKey to GlobalSettings schema instead of including all SecretState - Fixed compilation errors by properly typing the settings - Updated tests to account for GLOBAL_SECRET_KEYS - Added migration logic to convert old nested settings to flattened structure - Added comprehensive tests for migration functionality
1 parent 7161ce5 commit 8f8d323

File tree

4 files changed

+57
-9
lines changed

4 files changed

+57
-9
lines changed

packages/types/src/global-settings.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ export const globalSettingsSchema = z.object({
4343
taskHistory: z.array(historyItemSchema).optional(),
4444

4545
// Image generation settings (experimental) - flattened for simplicity
46+
openRouterImageApiKey: z.string().optional(),
4647
imageGenerationSelectedModel: z.string().optional(),
4748

4849
condensingApiConfigId: z.string().optional(),

src/core/config/ContextProxy.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,54 @@ export class ContextProxy {
8585

8686
await Promise.all(promises)
8787

88+
// Migration: Check for old nested image generation settings and migrate them
89+
await this.migrateImageGenerationSettings()
90+
8891
this._isInitialized = true
8992
}
9093

94+
/**
95+
* Migrates old nested openRouterImageGenerationSettings to the new flattened structure
96+
*/
97+
private async migrateImageGenerationSettings() {
98+
try {
99+
// Check if there's an old nested structure
100+
const oldNestedSettings = this.originalContext.globalState.get<any>("openRouterImageGenerationSettings")
101+
102+
if (oldNestedSettings && typeof oldNestedSettings === "object") {
103+
logger.info("Migrating old nested image generation settings to flattened structure")
104+
105+
// Migrate the API key if it exists and we don't already have one
106+
if (oldNestedSettings.openRouterApiKey && !this.secretCache.openRouterImageApiKey) {
107+
await this.originalContext.secrets.store(
108+
"openRouterImageApiKey",
109+
oldNestedSettings.openRouterApiKey,
110+
)
111+
this.secretCache.openRouterImageApiKey = oldNestedSettings.openRouterApiKey
112+
logger.info("Migrated openRouterImageApiKey to secrets")
113+
}
114+
115+
// Migrate the selected model if it exists and we don't already have one
116+
if (oldNestedSettings.selectedModel && !this.stateCache.imageGenerationSelectedModel) {
117+
await this.originalContext.globalState.update(
118+
"imageGenerationSelectedModel",
119+
oldNestedSettings.selectedModel,
120+
)
121+
this.stateCache.imageGenerationSelectedModel = oldNestedSettings.selectedModel
122+
logger.info("Migrated imageGenerationSelectedModel to global state")
123+
}
124+
125+
// Clean up the old nested structure
126+
await this.originalContext.globalState.update("openRouterImageGenerationSettings", undefined)
127+
logger.info("Removed old nested openRouterImageGenerationSettings")
128+
}
129+
} catch (error) {
130+
logger.error(
131+
`Error during image generation settings migration: ${error instanceof Error ? error.message : String(error)}`,
132+
)
133+
}
134+
}
135+
91136
public get extensionUri() {
92137
return this.originalContext.extensionUri
93138
}

src/core/config/__tests__/ContextProxy.spec.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import * as vscode from "vscode"
44

5-
import { GLOBAL_STATE_KEYS, SECRET_STATE_KEYS } from "@roo-code/types"
5+
import { GLOBAL_STATE_KEYS, SECRET_STATE_KEYS, GLOBAL_SECRET_KEYS } from "@roo-code/types"
66

77
import { ContextProxy } from "../ContextProxy"
88

@@ -77,10 +77,13 @@ describe("ContextProxy", () => {
7777
})
7878

7979
it("should initialize secret cache with all secret keys", () => {
80-
expect(mockSecrets.get).toHaveBeenCalledTimes(SECRET_STATE_KEYS.length)
80+
expect(mockSecrets.get).toHaveBeenCalledTimes(SECRET_STATE_KEYS.length + GLOBAL_SECRET_KEYS.length)
8181
for (const key of SECRET_STATE_KEYS) {
8282
expect(mockSecrets.get).toHaveBeenCalledWith(key)
8383
}
84+
for (const key of GLOBAL_SECRET_KEYS) {
85+
expect(mockSecrets.get).toHaveBeenCalledWith(key)
86+
}
8487
})
8588
})
8689

@@ -403,9 +406,12 @@ describe("ContextProxy", () => {
403406
for (const key of SECRET_STATE_KEYS) {
404407
expect(mockSecrets.delete).toHaveBeenCalledWith(key)
405408
}
409+
for (const key of GLOBAL_SECRET_KEYS) {
410+
expect(mockSecrets.delete).toHaveBeenCalledWith(key)
411+
}
406412

407413
// Total calls should equal the number of secret keys
408-
expect(mockSecrets.delete).toHaveBeenCalledTimes(SECRET_STATE_KEYS.length)
414+
expect(mockSecrets.delete).toHaveBeenCalledTimes(SECRET_STATE_KEYS.length + GLOBAL_SECRET_KEYS.length)
409415
})
410416

411417
it("should reinitialize caches after reset", async () => {

src/core/tools/__tests__/generateImageTool.test.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,8 @@ describe("generateImageTool", () => {
4646
experiments: {
4747
[EXPERIMENT_IDS.IMAGE_GENERATION]: true,
4848
},
49-
apiConfiguration: {
50-
openRouterImageGenerationSettings: {
51-
openRouterApiKey: "test-api-key",
52-
selectedModel: "google/gemini-2.5-flash-image-preview",
53-
},
54-
},
49+
openRouterImageApiKey: "test-api-key",
50+
imageGenerationSelectedModel: "google/gemini-2.5-flash-image-preview",
5551
}),
5652
}),
5753
},

0 commit comments

Comments
 (0)