feat(kilo-vscode): legacy migration wizard#6556
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| async function readLegacyProviderProfiles(context: vscode.ExtensionContext): Promise<LegacyProviderProfiles | null> { | ||
| const raw = await context.secrets.get(SECRET_KEY) | ||
| if (!raw) return null | ||
| const parsed = JSON.parse(raw) as Record<string, unknown> |
There was a problem hiding this comment.
[WARNING]: JSON.parse without error handling — corrupted SecretStorage data will throw an unhandled exception
readLegacyProviderProfiles calls JSON.parse(raw) without a try/catch. If the legacy SecretStorage entry contains malformed JSON (e.g. partial write, encoding issue, or a different extension wrote to the same key), this will throw and bubble up through detectLegacyData, potentially crashing the auto-detection on every extension startup.
The same issue exists in readLegacyMcpSettings at line 617.
Both functions should wrap the JSON.parse call and return null on failure, consistent with how parseCustomModesYaml already handles JSON parse errors gracefully.
| () => null, | ||
| ) | ||
| if (!bytes) return null | ||
| const parsed = JSON.parse(Buffer.from(bytes).toString("utf8")) as Record<string, unknown> |
There was a problem hiding this comment.
[WARNING]: Same JSON.parse without try/catch issue as readLegacyProviderProfiles above.
Corrupted or truncated mcp_settings.json on disk will throw an unhandled exception here. Should return null on parse failure.
| // Global allow with no specific command rules — apply immediately using the scalar form. | ||
| // PermissionConfig is "allow" | "ask" | "deny" | { read?: ..., ... }, so a global allow | ||
| // must be the scalar string, not an object with a "*" key. | ||
| await client.global.config.update({ config: { permission: "allow" } }) |
There was a problem hiding this comment.
[WARNING]: Scalar permission: "allow" will be overwritten by subsequent object update
When autoApprovalEnabled=true and there are no command lists, this line sets permission to the scalar "allow" via config.update. However, if the user also selected other permission groups (read, write, MCP, etc.), lines 508-509 will call config.update({ permission: { read: "allow", edit: "allow", ... } }) — an object that overwrites the scalar "allow", effectively downgrading the global allow to only the specific permissions in the object.
Consider either:
- Skipping the scalar update here and instead setting all individual permission keys to
"allow"in thepermissionobject, or - Short-circuiting: if global allow is set, skip the per-tool permission object update entirely and return early.
| message: error instanceof Error ? error.message : String(error), | ||
| }, | ||
| ], | ||
| }) |
There was a problem hiding this comment.
[SUGGESTION]: cachedLegacyData is never cleared after migration completes or is skipped
The cachedLegacyData field may contain sensitive data (provider API keys from the legacy profiles). After handleStartLegacyMigration completes or handleSkipLegacyMigration is called, consider setting this.cachedLegacyData = null to avoid keeping API keys in memory longer than necessary.
Code Review SummaryStatus: 5 Issues Found | Recommendation: Address before merge Fix these issues in Kilo Cloud Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (14 files)
|
|
are you sure the question functionality means the same? allow in the old extension means 'pick the first answer', is that the same to what it does in the new extension/cli? I think we might want to consider not migrating that if not. |
| } | ||
|
|
||
| // Follow-up question permissions | ||
| if (sel.questionPermission) { |
There was a problem hiding this comment.
as said in the other screen, are we sure that question permission means the same in the old and new extension?
| if (settings.enableChatAutocomplete !== undefined) { | ||
| await config.update("enableChatAutocomplete", settings.enableChatAutocomplete, vscode.ConfigurationTarget.Global) | ||
| } | ||
| // Only migrate snooze if the timestamp is still in the future |
There was a problem hiding this comment.
please dont migrate snooze
| return { item: "Autocomplete settings", category: "settings", status: "success" } | ||
| } | ||
|
|
||
| const SUPPORTED_LOCALES = new Set([ |
There was a problem hiding this comment.
there is some mapping needed, as languages in the old extension have another format. Did you test this?
| }, | ||
| lmstudio: { | ||
| id: "lmstudio", | ||
| key: "lmStudioBaseUrl", |
There was a problem hiding this comment.
[WARNING]: lmstudio mapping uses key: "lmStudioBaseUrl" — this is a URL, not an API key
LM Studio typically doesn't use API keys; it uses a base URL for local connections. Using the base URL as the key field means:
migrateProviderwill store the base URL as an auth credential viaclient.auth.set({ providerID: "lmstudio", auth: { type: "api", key: <base_url> } })- Since
urlFieldis also"lmStudioBaseUrl", the same value gets stored again inconfig.provider.lmstudio.options.apiKey(line 337 of migration-service.ts)
This provider likely needs special handling — either skip auth entirely and only migrate the base URL + model, or use a sentinel/empty key.
| const status = MigrationService.getMigrationStatus(this.extensionContext) | ||
| if (status) return // already prompted (skipped or completed) | ||
|
|
||
| this.migrationCheckInFlight = true |
There was a problem hiding this comment.
[WARNING]: migrationCheckInFlight is not reset if detectLegacyData throws
If detectLegacyData rejects (e.g. SecretStorage read fails), migrationCheckInFlight stays true forever because line 2028 is never reached. This permanently blocks the migration wizard from appearing.
Wrap in try/finally:
this.migrationCheckInFlight = true
try {
const data = await MigrationService.detectLegacyData(this.extensionContext)
// ... rest of logic
} finally {
this.migrationCheckInFlight = false
}| : undefined | ||
|
|
||
| const auth = organizationId | ||
| ? { type: "oauth" as const, access: apiKey, refresh: "", expires: 0, accountId: organizationId } |
There was a problem hiding this comment.
[WARNING]: OAuth auth for Kilo Gateway uses refresh: "" and expires: 0
The expires: 0 means the token appears immediately expired. If the CLI checks token expiry and attempts a refresh using the empty refresh token, this will fail. Verify that the CLI's auth layer treats expires: 0 as "no expiry" rather than "expired at epoch". If it interprets this as expired, the migrated Kilo Gateway credentials will be unusable.
Consider using a far-future timestamp (e.g. Date.now() + 365 * 24 * 60 * 60 * 1000) or a sentinel value that the CLI recognizes as "never expires".
| const prompt = [mode.roleDefinition, mode.customInstructions].filter(Boolean).join("\n\n") | ||
| return { | ||
| mode: "primary", | ||
| description: mode.customInstructions ?? mode.roleDefinition?.slice(0, 120), |
There was a problem hiding this comment.
[SUGGESTION]: description uses customInstructions which can be arbitrarily long
The LegacyCustomMode type has dedicated description and whenToUse fields that are more semantically appropriate for a short description. Using customInstructions (which is typically a long block of instructions) as the description may produce poor UX in the agent list.
Consider:
description: mode.description ?? mode.whenToUse ?? mode.roleDefinition?.slice(0, 120),| await Auth.set(providerID, info) | ||
| // kilocode_change start - invalidate provider/model cache after auth change | ||
| ModelCache.clear(providerID) | ||
| void Instance.disposeAll().catch(() => undefined) |
There was a problem hiding this comment.
[WARNING]: Instance.disposeAll() on every auth set/remove will cause N full instance disposals during migration
The migration service calls client.auth.set() in a loop for each selected provider (migration-service.ts line 330). Each call hits this endpoint, triggering Instance.disposeAll(). Migrating 5+ providers means 5+ rapid disposal cycles.
While disposeAll has a dedup guard for concurrent calls, sequential calls after each disposal completes will each trigger a full disposal. Consider:
- Adding a batch auth endpoint that accepts multiple provider credentials
- Or debouncing the
disposeAll()call (e.g. with a short timer) - Or having the migration service call a single "refresh" after all providers are migrated
markijbema
left a comment
markijbema
left a comment
There was a problem hiding this comment.
I'm not a 100% sure of all mappings, and i feel like we need good testing here to make sure it works like expected, because people do this once, and expect it to work
2 participants
Context
The new Kilo Code VS Code extension (
kilo-code NEW) coexists with the legacy v5.x extension. Users who installed the legacy extension have API keys, MCP server configurations, custom modes, and personal settings stored in VS Code SecretStorage and on disk. Without migration, switching to the new extension means losing all that configuration.This PR adds a guided migration wizard that detects legacy data on first run and walks the user through selecting what to import into the new CLI-backed extension.
Implementation
Detection
On first run (after the CLI connects and the webview is ready),
checkAndShowMigrationWizard()reads legacy data from:roo_cline_config_api_config) — provider profiles with API keys<globalStorage>/settings/mcp_settings.json) — MCP server configs<globalStorage>/settings/custom_modes.yaml) — custom agent modesIf any data is found and the user hasn't been prompted before, the webview navigates to the migration view and receives the detected data.
Migration Wizard (4-step flow)
What gets migrated
client.auth.set()per providerclient.global.config.update({ provider: { id: { options: { baseURL } } } })client.global.config.update({ mcp: {...} })client.global.config.update({ agent: {...} })as agent configsclient.global.config.update({ model: "provider/modelId" })client.global.config.update({ permission: {...} })— per-tool permission configkilo-code.new.languagekilo-code.new.autocomplete.*Auto-approval → Permission mapping
The legacy extension had a master toggle + 12 fine-grained
alwaysAllow*booleans. These are mapped to the CLI'sPermissionConfig:alwaysAllowReadOnly→read/glob/grep/list: "allow"alwaysAllowWrite→edit: "allow"alwaysAllowExecute+ command lists →bash: { "<cmd>": "allow"/"deny", "*": fallback }alwaysAllowMcp→skill: "allow"alwaysAllowModeSwitch/alwaysAllowSubtasks→task: "allow"alwaysAllowFollowupQuestions→question: "allow"alwaysAllowReadOnlyOutsideWorkspace→external_directory: "allow"permission: "allow"(global scalar)Provider mapping
provider-mapping.tsmaps 30+ legacy provider IDs to their new CLI equivalents, handling API key field names, base URL fields, and model ID fields per provider.Custom modes YAML parser
The legacy extension stored custom modes as YAML (with some versions using JSON). A minimal hand-rolled parser handles the specific YAML shape (
slug,name,roleDefinition,groups) without adding a runtime YAML dependency.Reliability fixes (from code review)
checkAndShowMigrationWizardis guarded by amigrationCheckInFlightboolean to prevent concurrent invocations (race betweenwebviewReadyandsse-connected)connectionState === "connected"so the wizard doesn't appear before providers/agents are loadedhandleStartLegacyMigrationis wrapped in try/catch — any network error sends alegacyMigrationCompletewith error status instead of leaving the progress screen stuckautoApprovalEnabled=true, no rules) uses the scalar"allow"form ofPermissionConfig, not an object with a"*"key"Re-run migration" entry point
AboutKiloCodeTabgets a "Migrate from legacy extension" button so users who skipped the initial prompt can re-run migration from Settings → About.Screenshots
How to Test
Prerequisites: Have the legacy Kilo Code v5.x extension installed with at least one configured provider, or manually populate globalState/SecretStorage with legacy data.
Auto-trigger (first run):
kilo.legacyMigrationStatusfrom globalState)Manual trigger:
Migration steps to verify:
~/.config/kilo/kilo.jsonccontains the migratedpermissionblock