Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/core/Cline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1059,7 +1059,7 @@ export class Cline extends EventEmitter<ClineEvents> {
async *attemptApiRequest(previousApiReqIndex: number, retryAttempt: number = 0): ApiStream {
let mcpHub: McpHub | undefined

const { mcpEnabled, alwaysApproveResubmit, requestDelaySeconds, rateLimitSeconds } =
const { mcpEnabled, alwaysApproveResubmit, requestDelaySeconds } =
(await this.providerRef.deref()?.getState()) ?? {}

let rateLimitDelay = 0
Expand All @@ -1068,7 +1068,8 @@ export class Cline extends EventEmitter<ClineEvents> {
if (this.lastApiRequestTime) {
const now = Date.now()
const timeSinceLastRequest = now - this.lastApiRequestTime
const rateLimit = rateLimitSeconds || 0
// Get rate limit from the current API configuration instead of global state
const rateLimit = this.apiConfiguration.rateLimitSeconds || 0
rateLimitDelay = Math.ceil(Math.max(0, rateLimit * 1000 - timeSinceLastRequest) / 1000)
}

Expand Down
57 changes: 53 additions & 4 deletions src/core/config/ConfigManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export class ConfigManager {
apiConfigs: {
default: {
id: this.generateId(),
rateLimitSeconds: 0, // Default rate limit is 0 seconds
},
},
}
Expand All @@ -43,6 +44,40 @@ export class ConfigManager {
/**
* Initialize config if it doesn't exist
*/
/**
* Migrate rate limit from global state to profile-specific setting
*/
async migrateRateLimitToProfiles(): Promise<void> {
try {
return await this.lock(async () => {
// Get the current global rate limit value
const globalRateLimit = this.context.globalState
? (await this.context.globalState.get<number>("rateLimitSeconds")) || 0
: 0

// Get all configurations
const config = await this.readConfig()

// Update each configuration with the global rate limit
for (const apiConfig of Object.values(config.apiConfigs)) {
apiConfig.rateLimitSeconds = globalRateLimit
}

// Save the updated configurations
await this.writeConfig(config)

// Remove the global rate limit setting
if (this.context.globalState) {
await this.context.globalState.update("rateLimitSeconds", undefined)
}

console.log(`[ConfigManager] Migrated global rate limit (${globalRateLimit}s) to all profiles`)
})
} catch (error) {
throw new Error(`Failed to migrate rate limit settings: ${error}`)
}
}

async initConfig(): Promise<void> {
try {
return await this.lock(async () => {
Expand All @@ -52,18 +87,27 @@ export class ConfigManager {
return
}

// Migrate: ensure all configs have IDs
let needsMigration = false
// Check if migration is needed for IDs
let needsIdMigration = false
for (const [name, apiConfig] of Object.entries(config.apiConfigs)) {
if (!apiConfig.id) {
apiConfig.id = this.generateId()
needsMigration = true
needsIdMigration = true
}
}

if (needsMigration) {
if (needsIdMigration) {
await this.writeConfig(config)
}

// Check if rate limit migration is needed
if (this.context.globalState) {
const hasGlobalRateLimit =
(await this.context.globalState.get<number>("rateLimitSeconds")) !== undefined
if (hasGlobalRateLimit) {
await this.migrateRateLimitToProfiles()
}
}
})
} catch (error) {
throw new Error(`Failed to initialize config: ${error}`)
Expand Down Expand Up @@ -99,6 +143,11 @@ export class ConfigManager {
currentConfig.apiConfigs[name] = {
...config,
id: existingConfig?.id || this.generateId(),
// Preserve rateLimitSeconds if not explicitly set in the new config
rateLimitSeconds:
config.rateLimitSeconds !== undefined
? config.rateLimitSeconds
: existingConfig?.rateLimitSeconds || 0,
}
await this.writeConfig(currentConfig)
})
Expand Down
131 changes: 131 additions & 0 deletions src/core/config/__tests__/ConfigManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ describe("ConfigManager", () => {
test: {
...newConfig,
id: testConfigId,
rateLimitSeconds: 0, // Default rate limit is 0 seconds
},
},
modeApiConfigs: {
Expand Down Expand Up @@ -216,6 +217,7 @@ describe("ConfigManager", () => {
apiProvider: "anthropic",
apiKey: "new-key",
id: "test-id",
rateLimitSeconds: 0, // Default rate limit is 0 seconds
},
},
}
Expand Down Expand Up @@ -489,4 +491,133 @@ describe("ConfigManager", () => {
)
})
})

describe("migrateRateLimitToProfiles", () => {
it("should migrate global rate limit to all profiles", async () => {
// Setup mock context with global rate limit
const mockGlobalState = {
get: jest.fn().mockResolvedValue(15), // Mock global rate limit of 15 seconds
update: jest.fn(),
}
;(mockContext as any).globalState = mockGlobalState

// Setup existing configs without rate limits
const existingConfig: ApiConfigData = {
currentApiConfigName: "default",
apiConfigs: {
default: {
id: "default",
apiProvider: "anthropic",
},
test: {
id: "test-id",
apiProvider: "openrouter",
},
},
}

mockSecrets.get.mockResolvedValue(JSON.stringify(existingConfig))

// Call the migration method
await configManager.migrateRateLimitToProfiles()

// Verify the configs were updated with the rate limit
const storedConfig = JSON.parse(mockSecrets.store.mock.calls[0][1])
expect(storedConfig.apiConfigs.default.rateLimitSeconds).toBe(15)
expect(storedConfig.apiConfigs.test.rateLimitSeconds).toBe(15)

// Verify the global rate limit was removed
expect(mockGlobalState.update).toHaveBeenCalledWith("rateLimitSeconds", undefined)
})

it("should handle empty config case", async () => {
// Create a new instance with fresh mocks for this test
jest.resetAllMocks()

const testMockContext = { ...mockContext }
const testMockSecrets = {
get: jest.fn(),
store: jest.fn(),
delete: jest.fn(),
onDidChange: jest.fn(),
}
testMockContext.secrets = testMockSecrets

// Setup mock context with global rate limit
const testMockGlobalState = {
get: jest.fn().mockResolvedValue(10), // Mock global rate limit of 10 seconds
update: jest.fn().mockResolvedValue(undefined),
keys: jest.fn().mockReturnValue([]),
setKeysForSync: jest.fn(),
}
testMockContext.globalState = testMockGlobalState

// Setup empty config
const emptyConfig: ApiConfigData = {
currentApiConfigName: "default",
apiConfigs: {},
}

// Mock the readConfig and writeConfig methods
testMockSecrets.get.mockResolvedValue(JSON.stringify(emptyConfig))
testMockSecrets.store.mockResolvedValue(undefined)

// Create a test instance that won't be affected by other tests
const testConfigManager = new ConfigManager(testMockContext as any)

// Override the lock method for this test
testConfigManager["_lock"] = Promise.resolve()
const originalLock = testConfigManager["lock"]
testConfigManager["lock"] = function (cb: () => Promise<any>) {
return cb()
}

// Call the migration method
await testConfigManager.migrateRateLimitToProfiles()

// Verify the global rate limit was removed even with empty config
expect(testMockGlobalState.update).toHaveBeenCalledWith("rateLimitSeconds", undefined)
})

it("should handle errors gracefully", async () => {
// Create a new instance with fresh mocks for this test
jest.resetAllMocks()

const testMockContext = { ...mockContext }
const testMockSecrets = {
get: jest.fn(),
store: jest.fn(),
delete: jest.fn(),
onDidChange: jest.fn(),
}
testMockContext.secrets = testMockSecrets

// Setup mock context with global rate limit
const testMockGlobalState = {
get: jest.fn().mockResolvedValue(5), // Mock global rate limit of 5 seconds
update: jest.fn().mockResolvedValue(undefined),
keys: jest.fn().mockReturnValue([]),
setKeysForSync: jest.fn(),
}
testMockContext.globalState = testMockGlobalState

// Force an error during read
testMockSecrets.get.mockRejectedValue(new Error("Storage failed"))

// Create a test instance that won't be affected by other tests
const testConfigManager = new ConfigManager(testMockContext as any)

// Override the lock method for this test
testConfigManager["_lock"] = Promise.resolve()
const originalLock = testConfigManager["lock"]
testConfigManager["lock"] = function (cb: () => Promise<any>) {
return Promise.reject(new Error("Failed to read config from secrets: Error: Storage failed"))
}

// Expect the migration to throw an error
await expect(testConfigManager.migrateRateLimitToProfiles()).rejects.toThrow(
"Failed to migrate rate limit settings: Error: Failed to read config from secrets: Error: Storage failed",
)
})
})
})
15 changes: 11 additions & 4 deletions src/core/webview/ClineProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1331,7 +1331,14 @@ export class ClineProvider implements vscode.WebviewViewProvider {
await this.postStateToWebview()
break
case "rateLimitSeconds":
await this.updateGlobalState("rateLimitSeconds", message.value ?? 0)
// Update the current API configuration with the rate limit value
const currentApiConfigName = (await this.getGlobalState("currentApiConfigName")) as string
if (currentApiConfigName) {
const apiConfig = await this.configManager.loadConfig(currentApiConfigName)
apiConfig.rateLimitSeconds = message.value ?? 0
await this.configManager.saveConfig(currentApiConfigName, apiConfig)
await this.updateApiConfiguration(apiConfig)
}
await this.postStateToWebview()
break
case "writeDelayMs":
Expand Down Expand Up @@ -2296,7 +2303,7 @@ export class ClineProvider implements vscode.WebviewViewProvider {
enableMcpServerCreation,
alwaysApproveResubmit,
requestDelaySeconds,
rateLimitSeconds,
// rateLimitSeconds is now part of apiConfiguration
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// rateLimitSeconds is now part of apiConfiguration

currentApiConfigName,
listApiConfigMeta,
mode,
Expand Down Expand Up @@ -2357,7 +2364,7 @@ export class ClineProvider implements vscode.WebviewViewProvider {
enableMcpServerCreation: enableMcpServerCreation ?? true,
alwaysApproveResubmit: alwaysApproveResubmit ?? false,
requestDelaySeconds: requestDelaySeconds ?? 10,
rateLimitSeconds: rateLimitSeconds ?? 0,
rateLimitSeconds: apiConfiguration.rateLimitSeconds ?? 0,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need this?

currentApiConfigName: currentApiConfigName ?? "default",
listApiConfigMeta: listApiConfigMeta ?? [],
mode: mode ?? defaultModeSlug,
Expand Down Expand Up @@ -2515,7 +2522,7 @@ export class ClineProvider implements vscode.WebviewViewProvider {
enableMcpServerCreation: stateValues.enableMcpServerCreation ?? true,
alwaysApproveResubmit: stateValues.alwaysApproveResubmit ?? false,
requestDelaySeconds: Math.max(5, stateValues.requestDelaySeconds ?? 10),
rateLimitSeconds: stateValues.rateLimitSeconds ?? 0,
// rateLimitSeconds is now part of the API configuration
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// rateLimitSeconds is now part of the API configuration

currentApiConfigName: stateValues.currentApiConfigName ?? "default",
listApiConfigMeta: stateValues.listApiConfigMeta ?? [],
modeApiConfigs: stateValues.modeApiConfigs ?? ({} as Record<Mode, string>),
Expand Down
2 changes: 1 addition & 1 deletion src/shared/ExtensionMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export interface ExtensionState {
alwaysAllowSubtasks?: boolean
browserToolEnabled?: boolean
requestDelaySeconds: number
rateLimitSeconds: number // Minimum time between successive requests (0 = disabled)
rateLimitSeconds?: number // Minimum time between successive requests (0 = disabled)
uriScheme?: string
currentTaskItem?: HistoryItem
allowedCommands?: string[]
Expand Down
2 changes: 2 additions & 0 deletions src/shared/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export interface ApiHandlerOptions {
export type ApiConfiguration = ApiHandlerOptions & {
apiProvider?: ApiProvider
id?: string // stable unique identifier
rateLimitSeconds?: number // Rate limit in seconds between API requests
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to add this here

}

// Import GlobalStateKey type from globalState.ts
Expand Down Expand Up @@ -130,6 +131,7 @@ export const API_CONFIG_KEYS: GlobalStateKey[] = [
"modelTemperature",
"modelMaxTokens",
"modelMaxThinkingTokens",
"rateLimitSeconds", // Added for profile-specific rate limiting
]

// Models
Expand Down
6 changes: 3 additions & 3 deletions webview-ui/src/components/settings/AdvancedSettings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { SectionHeader } from "./SectionHeader"
import { Section } from "./Section"

type AdvancedSettingsProps = HTMLAttributes<HTMLDivElement> & {
rateLimitSeconds: number
rateLimitSeconds?: number
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should move this out of the advanced settings and move it up near the model temperature so it's clear that it's linked to the profile

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'm going to place it below the temperature

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks!

diffEnabled?: boolean
fuzzyMatchThreshold?: number
setCachedStateField: SetCachedStateField<"rateLimitSeconds" | "diffEnabled" | "fuzzyMatchThreshold">
Expand Down Expand Up @@ -50,11 +50,11 @@ export const AdvancedSettings = ({
min="0"
max="60"
step="1"
value={rateLimitSeconds}
value={rateLimitSeconds || 0}
onChange={(e) => setCachedStateField("rateLimitSeconds", parseInt(e.target.value))}
className="h-2 focus:outline-0 w-4/5 accent-vscode-button-background"
/>
<span style={{ ...sliderLabelStyle }}>{rateLimitSeconds}s</span>
<span style={{ ...sliderLabelStyle }}>{rateLimitSeconds || 0}s</span>
</div>
</div>
<p className="text-vscode-descriptionForeground text-sm mt-0">
Expand Down
4 changes: 2 additions & 2 deletions webview-ui/src/components/settings/SettingsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone },
vscode.postMessage({ type: "mcpEnabled", bool: mcpEnabled })
vscode.postMessage({ type: "alwaysApproveResubmit", bool: alwaysApproveResubmit })
vscode.postMessage({ type: "requestDelaySeconds", value: requestDelaySeconds })
vscode.postMessage({ type: "rateLimitSeconds", value: rateLimitSeconds })
vscode.postMessage({ type: "rateLimitSeconds", value: rateLimitSeconds || 0 })
vscode.postMessage({ type: "maxOpenTabsContext", value: maxOpenTabsContext })
vscode.postMessage({ type: "maxWorkspaceFiles", value: maxWorkspaceFiles ?? 200 })
vscode.postMessage({ type: "showRooIgnoredFiles", bool: showRooIgnoredFiles })
Expand Down Expand Up @@ -440,7 +440,7 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone },

<div ref={advancedRef}>
<AdvancedSettings
rateLimitSeconds={rateLimitSeconds}
rateLimitSeconds={rateLimitSeconds || 0}
diffEnabled={diffEnabled}
fuzzyMatchThreshold={fuzzyMatchThreshold}
setCachedStateField={setCachedStateField}
Expand Down
2 changes: 1 addition & 1 deletion webview-ui/src/context/ExtensionStateContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export interface ExtensionStateContextType extends ExtensionState {
setAlwaysApproveResubmit: (value: boolean) => void
requestDelaySeconds: number
setRequestDelaySeconds: (value: number) => void
rateLimitSeconds: number
rateLimitSeconds?: number
setRateLimitSeconds: (value: number) => void
setCurrentApiConfigName: (value: string) => void
setListApiConfigMeta: (value: ApiConfigMeta[]) => void
Expand Down