Skip to content

Fix input box revert issue and configuration loss during profile switch #955 #979

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mrubens merged 6 commits into from
Feb 17, 2025
Merged

Fix input box revert issue and configuration loss during profile switch #955 #979

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c366a6e
Revert "Update API options on blur instead of on input"
System233 Feb 17, 2025
c4465de
Revert "Make sure to blur fields when user hits Done in settings"
System233 Feb 17, 2025
2dcde02
Revert "Add delay on submitting from welcome view"
System233 Feb 17, 2025
2943d65
Fix input box revert issue and configuration loss during profile swit…
System233 Feb 17, 2025
d69b910
Upgrade vscrui to 0.2.2 to fix the Dropdown component
System233 Feb 17, 2025
7316f82
Fix the `DialogContent/DialogTitle` error displayed in the console.
System233 Feb 17, 2025
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
153 changes: 90 additions & 63 deletions src/core/config/ConfigManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,29 +33,38 @@ export class ConfigManager {
return Math.random().toString(36).substring(2, 15)
}

// Synchronize readConfig/writeConfig operations to avoid data loss.
private _lock = Promise.resolve()
private lock<T>(cb: () => Promise<T>) {
const next = this._lock.then(cb)
this._lock = next.catch(() => {}) as Promise<void>
return next
}
Comment on lines +37 to +42
Copy link
Contributor

@samhvw8 samhvw8 Feb 17, 2025

Choose a reason for hiding this comment

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

this cool

/**
* Initialize config if it doesn't exist
*/
async initConfig(): Promise<void> {
try {
const config = await this.readConfig()
if (!config) {
await this.writeConfig(this.defaultConfig)
return
}
return await this.lock(async () => {
const config = await this.readConfig()
if (!config) {
await this.writeConfig(this.defaultConfig)
return
}

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

if (needsMigration) {
await this.writeConfig(config)
}
if (needsMigration) {
await this.writeConfig(config)
}
})
} catch (error) {
throw new Error(`Failed to initialize config: ${error}`)
}
Expand All @@ -66,12 +75,14 @@ export class ConfigManager {
*/
async listConfig(): Promise<ApiConfigMeta[]> {
try {
const config = await this.readConfig()
return Object.entries(config.apiConfigs).map(([name, apiConfig]) => ({
name,
id: apiConfig.id || "",
apiProvider: apiConfig.apiProvider,
}))
return await this.lock(async () => {
const config = await this.readConfig()
return Object.entries(config.apiConfigs).map(([name, apiConfig]) => ({
name,
id: apiConfig.id || "",
apiProvider: apiConfig.apiProvider,
}))
})
} catch (error) {
throw new Error(`Failed to list configs: ${error}`)
}
Expand All @@ -82,13 +93,15 @@ export class ConfigManager {
*/
async saveConfig(name: string, config: ApiConfiguration): Promise<void> {
try {
const currentConfig = await this.readConfig()
const existingConfig = currentConfig.apiConfigs[name]
currentConfig.apiConfigs[name] = {
...config,
id: existingConfig?.id || this.generateId(),
}
await this.writeConfig(currentConfig)
return await this.lock(async () => {
const currentConfig = await this.readConfig()
const existingConfig = currentConfig.apiConfigs[name]
currentConfig.apiConfigs[name] = {
...config,
id: existingConfig?.id || this.generateId(),
}
await this.writeConfig(currentConfig)
})
} catch (error) {
throw new Error(`Failed to save config: ${error}`)
}
Expand All @@ -99,17 +112,19 @@ export class ConfigManager {
*/
async loadConfig(name: string): Promise<ApiConfiguration> {
try {
const config = await this.readConfig()
const apiConfig = config.apiConfigs[name]
return await this.lock(async () => {
const config = await this.readConfig()
const apiConfig = config.apiConfigs[name]

if (!apiConfig) {
throw new Error(`Config '${name}' not found`)
}
if (!apiConfig) {
throw new Error(`Config '${name}' not found`)
}

config.currentApiConfigName = name
await this.writeConfig(config)
config.currentApiConfigName = name
await this.writeConfig(config)

return apiConfig
return apiConfig
})
} catch (error) {
throw new Error(`Failed to load config: ${error}`)
}
Expand All @@ -120,18 +135,20 @@ export class ConfigManager {
*/
async deleteConfig(name: string): Promise<void> {
try {
const currentConfig = await this.readConfig()
if (!currentConfig.apiConfigs[name]) {
throw new Error(`Config '${name}' not found`)
}
return await this.lock(async () => {
const currentConfig = await this.readConfig()
if (!currentConfig.apiConfigs[name]) {
throw new Error(`Config '${name}' not found`)
}

// Don't allow deleting the default config
if (Object.keys(currentConfig.apiConfigs).length === 1) {
throw new Error(`Cannot delete the last remaining configuration.`)
}
// Don't allow deleting the default config
if (Object.keys(currentConfig.apiConfigs).length === 1) {
throw new Error(`Cannot delete the last remaining configuration.`)
}

delete currentConfig.apiConfigs[name]
await this.writeConfig(currentConfig)
delete currentConfig.apiConfigs[name]
await this.writeConfig(currentConfig)
})
} catch (error) {
throw new Error(`Failed to delete config: ${error}`)
}
Expand All @@ -142,13 +159,15 @@ export class ConfigManager {
*/
async setCurrentConfig(name: string): Promise<void> {
try {
const currentConfig = await this.readConfig()
if (!currentConfig.apiConfigs[name]) {
throw new Error(`Config '${name}' not found`)
}
return await this.lock(async () => {
const currentConfig = await this.readConfig()
if (!currentConfig.apiConfigs[name]) {
throw new Error(`Config '${name}' not found`)
}

currentConfig.currentApiConfigName = name
await this.writeConfig(currentConfig)
currentConfig.currentApiConfigName = name
await this.writeConfig(currentConfig)
})
} catch (error) {
throw new Error(`Failed to set current config: ${error}`)
}
Expand All @@ -159,8 +178,10 @@ export class ConfigManager {
*/
async hasConfig(name: string): Promise<boolean> {
try {
const config = await this.readConfig()
return name in config.apiConfigs
return await this.lock(async () => {
const config = await this.readConfig()
return name in config.apiConfigs
})
} catch (error) {
throw new Error(`Failed to check config existence: ${error}`)
}
Expand All @@ -171,12 +192,14 @@ export class ConfigManager {
*/
async setModeConfig(mode: Mode, configId: string): Promise<void> {
try {
const currentConfig = await this.readConfig()
if (!currentConfig.modeApiConfigs) {
currentConfig.modeApiConfigs = {}
}
currentConfig.modeApiConfigs[mode] = configId
await this.writeConfig(currentConfig)
return await this.lock(async () => {
const currentConfig = await this.readConfig()
if (!currentConfig.modeApiConfigs) {
currentConfig.modeApiConfigs = {}
}
currentConfig.modeApiConfigs[mode] = configId
await this.writeConfig(currentConfig)
})
} catch (error) {
throw new Error(`Failed to set mode config: ${error}`)
}
Expand All @@ -187,8 +210,10 @@ export class ConfigManager {
*/
async getModeConfigId(mode: Mode): Promise<string | undefined> {
try {
const config = await this.readConfig()
return config.modeApiConfigs?.[mode]
return await this.lock(async () => {
const config = await this.readConfig()
return config.modeApiConfigs?.[mode]
})
} catch (error) {
throw new Error(`Failed to get mode config: ${error}`)
}
Expand All @@ -205,7 +230,9 @@ export class ConfigManager {
* Reset all configuration by deleting the stored config from secrets
*/
public async resetAllConfigs(): Promise<void> {
await this.context.secrets.delete(this.getConfigKey())
return await this.lock(async () => {
await this.context.secrets.delete(this.getConfigKey())
})
}

private async readConfig(): Promise<ApiConfigData> {
Expand Down
110 changes: 63 additions & 47 deletions src/core/webview/ClineProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1317,6 +1317,20 @@ export class ClineProvider implements vscode.WebviewViewProvider {
}
break
}
case "saveApiConfiguration":
if (message.text && message.apiConfiguration) {
try {
await this.configManager.saveConfig(message.text, message.apiConfiguration)
const listApiConfig = await this.configManager.listConfig()
await this.updateGlobalState("listApiConfigMeta", listApiConfig)
} catch (error) {
this.outputChannel.appendLine(
`Error save api configuration: ${JSON.stringify(error, Object.getOwnPropertyNames(error), 2)}`,
)
vscode.window.showErrorMessage("Failed to save api configuration")
}
}
break
case "upsertApiConfiguration":
Copy link
Contributor

@samhvw8 samhvw8 Feb 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@mrubens @System233 i have an idea that remove upsertApiConfiguration and replace it with saveApiConfiguration,
we will have a little bit more change, on handleSubmit we will add 1 more loadApiConfiguration to that section, so we will decouple save & load api

Copy link
Contributor

@samhvw8 samhvw8 Feb 14, 2025

Choose a reason for hiding this comment

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

@System233 @mrubens like this System233#1 ?

if (message.text && message.apiConfiguration) {
try {
Expand Down Expand Up @@ -1361,9 +1375,9 @@ export class ClineProvider implements vscode.WebviewViewProvider {
await this.postStateToWebview()
} catch (error) {
this.outputChannel.appendLine(
`Error create new api configuration: ${JSON.stringify(error, Object.getOwnPropertyNames(error), 2)}`,
`Error rename api configuration: ${JSON.stringify(error, Object.getOwnPropertyNames(error), 2)}`,
)
vscode.window.showErrorMessage("Failed to create api configuration")
vscode.window.showErrorMessage("Failed to rename api configuration")
}
}
break
Expand Down Expand Up @@ -1647,51 +1661,53 @@ export class ClineProvider implements vscode.WebviewViewProvider {
requestyModelInfo,
modelTemperature,
} = apiConfiguration
await this.updateGlobalState("apiProvider", apiProvider)
await this.updateGlobalState("apiModelId", apiModelId)
await this.storeSecret("apiKey", apiKey)
await this.updateGlobalState("glamaModelId", glamaModelId)
await this.updateGlobalState("glamaModelInfo", glamaModelInfo)
await this.storeSecret("glamaApiKey", glamaApiKey)
await this.storeSecret("openRouterApiKey", openRouterApiKey)
await this.storeSecret("awsAccessKey", awsAccessKey)
await this.storeSecret("awsSecretKey", awsSecretKey)
await this.storeSecret("awsSessionToken", awsSessionToken)
await this.updateGlobalState("awsRegion", awsRegion)
await this.updateGlobalState("awsUseCrossRegionInference", awsUseCrossRegionInference)
await this.updateGlobalState("awsProfile", awsProfile)
await this.updateGlobalState("awsUseProfile", awsUseProfile)
await this.updateGlobalState("vertexProjectId", vertexProjectId)
await this.updateGlobalState("vertexRegion", vertexRegion)
await this.updateGlobalState("openAiBaseUrl", openAiBaseUrl)
await this.storeSecret("openAiApiKey", openAiApiKey)
await this.updateGlobalState("openAiModelId", openAiModelId)
await this.updateGlobalState("openAiCustomModelInfo", openAiCustomModelInfo)
await this.updateGlobalState("openAiUseAzure", openAiUseAzure)
await this.updateGlobalState("ollamaModelId", ollamaModelId)
await this.updateGlobalState("ollamaBaseUrl", ollamaBaseUrl)
await this.updateGlobalState("lmStudioModelId", lmStudioModelId)
await this.updateGlobalState("lmStudioBaseUrl", lmStudioBaseUrl)
await this.updateGlobalState("anthropicBaseUrl", anthropicBaseUrl)
await this.storeSecret("geminiApiKey", geminiApiKey)
await this.storeSecret("openAiNativeApiKey", openAiNativeApiKey)
await this.storeSecret("deepSeekApiKey", deepSeekApiKey)
await this.updateGlobalState("azureApiVersion", azureApiVersion)
await this.updateGlobalState("openAiStreamingEnabled", openAiStreamingEnabled)
await this.updateGlobalState("openRouterModelId", openRouterModelId)
await this.updateGlobalState("openRouterModelInfo", openRouterModelInfo)
await this.updateGlobalState("openRouterBaseUrl", openRouterBaseUrl)
await this.updateGlobalState("openRouterUseMiddleOutTransform", openRouterUseMiddleOutTransform)
await this.updateGlobalState("vsCodeLmModelSelector", vsCodeLmModelSelector)
await this.storeSecret("mistralApiKey", mistralApiKey)
await this.updateGlobalState("mistralCodestralUrl", mistralCodestralUrl)
await this.storeSecret("unboundApiKey", unboundApiKey)
await this.updateGlobalState("unboundModelId", unboundModelId)
await this.updateGlobalState("unboundModelInfo", unboundModelInfo)
await this.storeSecret("requestyApiKey", requestyApiKey)
await this.updateGlobalState("requestyModelId", requestyModelId)
await this.updateGlobalState("requestyModelInfo", requestyModelInfo)
await this.updateGlobalState("modelTemperature", modelTemperature)
await Promise.all([
Copy link
Collaborator

@mrubens mrubens Feb 17, 2025

Choose a reason for hiding this comment

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

Great catch here

this.updateGlobalState("apiProvider", apiProvider),
this.updateGlobalState("apiModelId", apiModelId),
this.storeSecret("apiKey", apiKey),
this.updateGlobalState("glamaModelId", glamaModelId),
this.updateGlobalState("glamaModelInfo", glamaModelInfo),
this.storeSecret("glamaApiKey", glamaApiKey),
this.storeSecret("openRouterApiKey", openRouterApiKey),
this.storeSecret("awsAccessKey", awsAccessKey),
this.storeSecret("awsSecretKey", awsSecretKey),
this.storeSecret("awsSessionToken", awsSessionToken),
this.updateGlobalState("awsRegion", awsRegion),
this.updateGlobalState("awsUseCrossRegionInference", awsUseCrossRegionInference),
this.updateGlobalState("awsProfile", awsProfile),
this.updateGlobalState("awsUseProfile", awsUseProfile),
this.updateGlobalState("vertexProjectId", vertexProjectId),
this.updateGlobalState("vertexRegion", vertexRegion),
this.updateGlobalState("openAiBaseUrl", openAiBaseUrl),
this.storeSecret("openAiApiKey", openAiApiKey),
this.updateGlobalState("openAiModelId", openAiModelId),
this.updateGlobalState("openAiCustomModelInfo", openAiCustomModelInfo),
this.updateGlobalState("openAiUseAzure", openAiUseAzure),
this.updateGlobalState("ollamaModelId", ollamaModelId),
this.updateGlobalState("ollamaBaseUrl", ollamaBaseUrl),
this.updateGlobalState("lmStudioModelId", lmStudioModelId),
this.updateGlobalState("lmStudioBaseUrl", lmStudioBaseUrl),
this.updateGlobalState("anthropicBaseUrl", anthropicBaseUrl),
this.storeSecret("geminiApiKey", geminiApiKey),
this.storeSecret("openAiNativeApiKey", openAiNativeApiKey),
this.storeSecret("deepSeekApiKey", deepSeekApiKey),
this.updateGlobalState("azureApiVersion", azureApiVersion),
this.updateGlobalState("openAiStreamingEnabled", openAiStreamingEnabled),
this.updateGlobalState("openRouterModelId", openRouterModelId),
this.updateGlobalState("openRouterModelInfo", openRouterModelInfo),
this.updateGlobalState("openRouterBaseUrl", openRouterBaseUrl),
this.updateGlobalState("openRouterUseMiddleOutTransform", openRouterUseMiddleOutTransform),
this.updateGlobalState("vsCodeLmModelSelector", vsCodeLmModelSelector),
this.storeSecret("mistralApiKey", mistralApiKey),
this.updateGlobalState("mistralCodestralUrl", mistralCodestralUrl),
this.storeSecret("unboundApiKey", unboundApiKey),
this.updateGlobalState("unboundModelId", unboundModelId),
this.updateGlobalState("unboundModelInfo", unboundModelInfo),
this.storeSecret("requestyApiKey", requestyApiKey),
this.updateGlobalState("requestyModelId", requestyModelId),
this.updateGlobalState("requestyModelInfo", requestyModelInfo),
this.updateGlobalState("modelTemperature", modelTemperature),
])
if (this.cline) {
this.cline.api = buildApiHandler(apiConfiguration)
}
Expand Down
Loading
Loading