Skip to content

Commit 1356f0d

Browse files
olweraltuveolweraltuve
committed
feat(config): implement rate limit migration for API configurations
Add support for migrating global rate limit settings to individual API profiles. Introduce a migrations object to track migration status and apply rate limits during configuration saves. New tests ensure correct migration behavior for existing and new profiles.
1 parent 2f4873a commit 1356f0d

File tree

2 files changed

+255
-1
lines changed

2 files changed

+255
-1
lines changed

‎src/core/config/ConfigManager.ts‎

Lines changed: 100 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ export interface ApiConfigData {
99
[key: string]: ApiConfiguration
1010
}
1111
modeApiConfigs?: Partial<Record<Mode, string>>
12+
migrations?: {
13+
rateLimitMigrated?: boolean // Flag to track if rate limit migration has been applied
14+
}
1215
}
1316

1417
export class ConfigManager {
@@ -17,8 +20,12 @@ export class ConfigManager {
1720
apiConfigs: {
1821
default: {
1922
id: this.generateId(),
23+
rateLimitSeconds: 0, // Set default rate limit for new installations
2024
},
2125
},
26+
migrations: {
27+
rateLimitMigrated: true, // Mark as migrated for fresh installs
28+
},
2229
}
2330

2431
private readonly SCOPE_PREFIX = "roo_cline_config_"
@@ -52,15 +59,28 @@ export class ConfigManager {
5259
return
5360
}
5461

55-
// Migrate: ensure all configs have IDs
62+
// Initialize migrations tracking object if it doesn't exist
63+
if (!config.migrations) {
64+
config.migrations = {}
65+
}
66+
5667
let needsMigration = false
68+
69+
// Migrate: ensure all configs have IDs
5770
for (const [name, apiConfig] of Object.entries(config.apiConfigs)) {
5871
if (!apiConfig.id) {
5972
apiConfig.id = this.generateId()
6073
needsMigration = true
6174
}
6275
}
6376

77+
// Apply rate limit migration if needed
78+
if (!config.migrations.rateLimitMigrated) {
79+
await this.migrateRateLimit(config)
80+
config.migrations.rateLimitMigrated = true
81+
needsMigration = true
82+
}
83+
6484
if (needsMigration) {
6585
await this.writeConfig(config)
6686
}
@@ -70,6 +90,43 @@ export class ConfigManager {
7090
}
7191
}
7292

93+
/**
94+
* Migrate rate limit settings from global state to per-profile configuration
95+
*/
96+
private async migrateRateLimit(config: ApiConfigData): Promise<void> {
97+
try {
98+
// Get the global rate limit value from extension state
99+
let rateLimitSeconds: number | undefined
100+
101+
try {
102+
// Try to get global state rate limit
103+
rateLimitSeconds = await this.context.globalState.get<number>("rateLimitSeconds")
104+
console.log(`[RateLimitMigration] Found global rate limit value: ${rateLimitSeconds}`)
105+
} catch (error) {
106+
console.error("[RateLimitMigration] Error getting global rate limit:", error)
107+
}
108+
109+
// If no global rate limit, use default value of 5 seconds
110+
if (rateLimitSeconds === undefined) {
111+
rateLimitSeconds = 5 // Default value
112+
console.log(`[RateLimitMigration] Using default rate limit value: ${rateLimitSeconds}`)
113+
}
114+
115+
// Apply the rate limit to all API configurations
116+
for (const [name, apiConfig] of Object.entries(config.apiConfigs)) {
117+
// Only set if not already configured
118+
if (apiConfig.rateLimitSeconds === undefined) {
119+
console.log(`[RateLimitMigration] Applying rate limit ${rateLimitSeconds}s to profile: ${name}`)
120+
apiConfig.rateLimitSeconds = rateLimitSeconds
121+
}
122+
}
123+
124+
console.log(`[RateLimitMigration] Migration complete`)
125+
} catch (error) {
126+
console.error(`[RateLimitMigration] Failed to migrate rate limit settings:`, error)
127+
}
128+
}
129+
73130
/**
74131
* List all available configs with metadata
75132
*/
@@ -96,6 +153,48 @@ export class ConfigManager {
96153
return await this.lock(async () => {
97154
const currentConfig = await this.readConfig()
98155
const existingConfig = currentConfig.apiConfigs[name]
156+
157+
// If this is a new config or doesn't have rate limit, try to apply the global rate limit
158+
if (!existingConfig || config.rateLimitSeconds === undefined) {
159+
// Apply rate limit if not specified explicitly in the config being saved
160+
if (config.rateLimitSeconds === undefined) {
161+
let globalRateLimit: number | undefined
162+
163+
// First check if we have an existing migrated config to copy from
164+
const anyExistingConfig = Object.values(currentConfig.apiConfigs)[0]
165+
if (anyExistingConfig?.rateLimitSeconds !== undefined) {
166+
globalRateLimit = anyExistingConfig.rateLimitSeconds
167+
console.log(
168+
`[RateLimitMigration] Using existing profile's rate limit value: ${globalRateLimit}s`,
169+
)
170+
} else {
171+
// Otherwise check global state
172+
try {
173+
globalRateLimit = await this.context.globalState.get<number>("rateLimitSeconds")
174+
console.log(
175+
`[RateLimitMigration] Using global rate limit for new profile: ${globalRateLimit}s`,
176+
)
177+
} catch (error) {
178+
console.error(
179+
"[RateLimitMigration] Error getting global rate limit for new profile:",
180+
error,
181+
)
182+
}
183+
184+
// Use default if not found
185+
if (globalRateLimit === undefined) {
186+
globalRateLimit = 5 // Default value
187+
console.log(
188+
`[RateLimitMigration] Using default rate limit value for new profile: ${globalRateLimit}s`,
189+
)
190+
}
191+
}
192+
193+
// Apply the rate limit to the new config
194+
config.rateLimitSeconds = globalRateLimit
195+
}
196+
}
197+
99198
currentConfig.apiConfigs[name] = {
100199
...config,
101200
id: existingConfig?.id || this.generateId(),

‎src/core/config/__tests__/rateLimitMigration.test.ts‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
import { ConfigManager } from "../ConfigManager"
2+
import { ExtensionContext } from "vscode"
3+
import path from "path"
4+
import fs from "fs"
5+
6+
// Mock class to simulate VSCode extension context
7+
class MockExtensionContext {
8+
private _globalState: Record<string, any>
9+
private _secrets: Record<string, any>
10+
private _storageUri: { fsPath: string }
11+
private _globalStorageUri: { fsPath: string }
12+
13+
constructor(initialGlobalState = {}, initialSecrets = {}) {
14+
this._globalState = initialGlobalState
15+
this._secrets = initialSecrets
16+
this._storageUri = { fsPath: path.join(__dirname, "mock-storage") }
17+
this._globalStorageUri = { fsPath: path.join(__dirname, "mock-global-storage") }
18+
}
19+
20+
get globalState() {
21+
return {
22+
get: jest.fn().mockImplementation((key) => Promise.resolve(this._globalState[key])),
23+
update: jest.fn().mockImplementation((key, value) => {
24+
this._globalState[key] = value
25+
return Promise.resolve()
26+
}),
27+
}
28+
}
29+
30+
get secrets() {
31+
return {
32+
get: jest.fn().mockImplementation((key) => Promise.resolve(this._secrets[key])),
33+
store: jest.fn().mockImplementation((key, value) => {
34+
this._secrets[key] = value
35+
return Promise.resolve()
36+
}),
37+
delete: jest.fn().mockImplementation((key) => {
38+
delete this._secrets[key]
39+
return Promise.resolve()
40+
}),
41+
}
42+
}
43+
44+
get storageUri() {
45+
return this._storageUri
46+
}
47+
48+
get globalStorageUri() {
49+
return this._globalStorageUri
50+
}
51+
}
52+
53+
describe("Rate Limit Migration Tests", () => {
54+
beforeEach(() => {
55+
jest.clearAllMocks()
56+
})
57+
58+
it("should migrate existing global rate limit to all profiles", async () => {
59+
// Case 1: Test migration with existing global rate limit
60+
const context1 = new MockExtensionContext(
61+
{ rateLimitSeconds: 10 }, // Global state with rateLimitSeconds set to 10
62+
{
63+
roo_cline_config_api_config: JSON.stringify({
64+
currentApiConfigName: "default",
65+
apiConfigs: {
66+
default: { id: "abc123" },
67+
profile1: { id: "def456", apiProvider: "anthropic" },
68+
profile2: { id: "ghi789", apiProvider: "openrouter" },
69+
},
70+
}),
71+
},
72+
)
73+
74+
// Use a type assertion to bypass TypeScript's type checking
75+
const configManager1 = new ConfigManager(context1 as unknown as ExtensionContext)
76+
77+
// Wait for initialization to complete
78+
await new Promise((resolve) => setTimeout(resolve, 100))
79+
80+
// Check if the migration was applied
81+
const config1 = JSON.parse((await context1.secrets.get("roo_cline_config_api_config")) as string)
82+
83+
// Verify migrations flag is set
84+
expect(config1.migrations.rateLimitMigrated).toBeTruthy()
85+
86+
// Verify rate limits were applied to all profiles
87+
expect(config1.apiConfigs.default.rateLimitSeconds).toBe(10)
88+
expect(config1.apiConfigs.profile1.rateLimitSeconds).toBe(10)
89+
expect(config1.apiConfigs.profile2.rateLimitSeconds).toBe(10)
90+
})
91+
92+
it("should use default rate limit when no global rate limit exists", async () => {
93+
// Case 2: Test migration without global rate limit (should use default value)
94+
const context2 = new MockExtensionContext(
95+
{}, // No global state
96+
{
97+
roo_cline_config_api_config: JSON.stringify({
98+
currentApiConfigName: "default",
99+
apiConfigs: {
100+
default: { id: "abc123" },
101+
profile1: { id: "def456", apiProvider: "anthropic" },
102+
},
103+
}),
104+
},
105+
)
106+
107+
// Use a type assertion to bypass TypeScript's type checking
108+
const configManager2 = new ConfigManager(context2 as unknown as ExtensionContext)
109+
110+
// Wait for initialization to complete
111+
await new Promise((resolve) => setTimeout(resolve, 100))
112+
113+
// Check if the migration was applied with default value
114+
const config2 = JSON.parse((await context2.secrets.get("roo_cline_config_api_config")) as string)
115+
116+
// Verify migrations flag is set
117+
expect(config2.migrations.rateLimitMigrated).toBeTruthy()
118+
119+
// Verify default rate limits were applied
120+
expect(config2.apiConfigs.default.rateLimitSeconds).toBe(5) // Default is 5
121+
expect(config2.apiConfigs.profile1.rateLimitSeconds).toBe(5) // Default is 5
122+
})
123+
124+
it("should apply rate limit to newly created profiles", async () => {
125+
// Case 3: Test creating new profile via saveConfig
126+
const context3 = new MockExtensionContext(
127+
{ rateLimitSeconds: 15 }, // Global state with rateLimitSeconds set to 15
128+
{
129+
roo_cline_config_api_config: JSON.stringify({
130+
currentApiConfigName: "default",
131+
apiConfigs: {
132+
default: { id: "abc123", rateLimitSeconds: 8 },
133+
},
134+
migrations: { rateLimitMigrated: true },
135+
}),
136+
},
137+
)
138+
139+
// Use a type assertion to bypass TypeScript's type checking
140+
const configManager3 = new ConfigManager(context3 as unknown as ExtensionContext)
141+
142+
// Wait for initialization to complete
143+
await new Promise((resolve) => setTimeout(resolve, 100))
144+
145+
// Save a new profile
146+
await configManager3.saveConfig("newProfile", { apiProvider: "anthropic" })
147+
148+
// Check if the new profile got a rate limit from existing profiles
149+
const config3 = JSON.parse((await context3.secrets.get("roo_cline_config_api_config")) as string)
150+
151+
// The new profile should inherit rate limit from existing profiles (8s)
152+
expect(config3.apiConfigs.newProfile.rateLimitSeconds).toBe(8)
153+
expect(config3.apiConfigs.newProfile.apiProvider).toBe("anthropic")
154+
})
155+
})

0 commit comments

Comments
 (0)