Skip to content
Merged
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: 5 additions & 0 deletions .changeset/smooth-months-search.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"kilo-code": patch
---

Fix for double id's stored in profiles when activating a new profile and then adding a new one
107 changes: 103 additions & 4 deletions src/core/config/ProviderSettingsManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,16 @@ export class ProviderSettingsManager {
},
}

// kilocode_change start
private pendingDuplicateIdRepairReport: Record<string, string[]> | null = null

public consumeDuplicateIdRepairReport(): Record<string, string[]> | null {
const report = this.pendingDuplicateIdRepairReport
this.pendingDuplicateIdRepairReport = null
return report
}
// kilocode_change end

private readonly context: ExtensionContext

constructor(context: ExtensionContext) {
Expand All @@ -99,6 +109,16 @@ export class ProviderSettingsManager {
public initialize(): Promise<void> {
return this.initialization
}

private generateUniqueId(existingIds: Set<string>): string {
let id: string
do {
id = this.generateId()
} while (existingIds.has(id))

existingIds.add(id)
return id
}
// kilocode_change end

public generateId() {
Expand Down Expand Up @@ -151,6 +171,75 @@ export class ProviderSettingsManager {
}
}

// kilocode_change start: Repair duplicated IDs (keep the first occurrence based on apiConfigs insertion order).
const existingIds = new Set(
Object.values(providerProfiles.apiConfigs)
.map((c) => c.id)
.filter((id): id is string => Boolean(id)),
)

const seenIds = new Set<string>()
let updatedCloudProfileIds: Set<string> | undefined
const duplicateIdRepairReport: Record<string, string[]> = {}

for (const [name, apiConfig] of Object.entries(providerProfiles.apiConfigs)) {
const id = apiConfig.id
if (!id) continue

// first profile keeps its id
if (!seenIds.has(id)) {
seenIds.add(id)
continue
}

const newId = this.generateUniqueId(existingIds)
apiConfig.id = newId
isDirty = true

duplicateIdRepairReport[id] ??= []
duplicateIdRepairReport[id].push(newId)

// If this was considered cloud-managed before (by virtue of its old id being listed),
// keep it cloud-managed by adding the new id as well.
if (providerProfiles.cloudProfileIds?.includes(id)) {
updatedCloudProfileIds ??= new Set(providerProfiles.cloudProfileIds)
updatedCloudProfileIds.add(newId)
}

console.warn(
`[ProviderSettingsManager] Deduped duplicate provider profile id '${id}' for '${name}', new id '${newId}'`,
)
}

if (updatedCloudProfileIds) {
providerProfiles.cloudProfileIds = Array.from(updatedCloudProfileIds)
isDirty = true
}

if (Object.keys(duplicateIdRepairReport).length > 0) {
this.pendingDuplicateIdRepairReport = duplicateIdRepairReport
}

// Keep secrets-side references consistent (post-dedupe).
const validProfileIds = new Set(
Object.values(providerProfiles.apiConfigs)
.map((c) => c.id)
.filter((id): id is string => Boolean(id)),
)

const firstProfileId = Object.values(providerProfiles.apiConfigs)[0]?.id

// Fix modeApiConfigs stored inside providerProfiles (secrets) if they point to a missing id.
if (providerProfiles.modeApiConfigs && firstProfileId) {
for (const [mode, configId] of Object.entries(providerProfiles.modeApiConfigs)) {
if (!validProfileIds.has(configId)) {
providerProfiles.modeApiConfigs[mode] = firstProfileId
isDirty = true
}
}
}
// kilocode_Change end

// Ensure migrations field exists
if (!providerProfiles.migrations) {
providerProfiles.migrations = {
Expand Down Expand Up @@ -426,12 +515,22 @@ export class ProviderSettingsManager {
return await this.lock(async () => {
const providerProfiles = await this.load()

// kilocode_change - autocomplete profile type system
// kilocode_change start" autocomplete profile type system and check for duplicate id's
await this.validateAutocompleteConstraint(providerProfiles, name, config.profileType)

// Preserve the existing ID if this is an update to an existing config.
const existingId = providerProfiles.apiConfigs[name]?.id
const id = config.id || existingId || this.generateId()
const existingEntry = providerProfiles.apiConfigs[name]
const existingIds = new Set(
Object.values(providerProfiles.apiConfigs)
.map((c) => c.id)
.filter((id): id is string => Boolean(id)),
)

// EXISTING: preserve stored id; NEW: generate fresh unique id.
const id =
existingEntry?.id && existingEntry.id.length > 0
? existingEntry.id
: this.generateUniqueId(existingIds)
// kilocode_change end

// Filter out settings from other providers.
const filteredConfig = discriminatedProviderSettingsWithIdSchema.parse(config)
Expand Down
75 changes: 75 additions & 0 deletions src/core/config/__tests__/ProviderSettingsManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,81 @@ describe("ProviderSettingsManager", () => {
expect(storedConfig.apiConfigs.test.id).toBeTruthy()
})

// kilocode_change start
it("should not change anything when no duplicated ids exist", async () => {
mockSecrets.get.mockResolvedValue(
JSON.stringify({
currentApiConfigName: "first",
apiConfigs: {
first: { id: "id-1", apiProvider: "anthropic" },
second: { id: "id-2", apiProvider: "anthropic" },
},
modeApiConfigs: {},
migrations: {
rateLimitSecondsMigrated: true,
diffSettingsMigrated: true,
openAiHeadersMigrated: true,
consecutiveMistakeLimitMigrated: true,
todoListEnabledMigrated: true,
morphApiKeyMigrated: true,
},
}),
)

await providerSettingsManager.initialize()

// No changes expected since there are no duplicates and migrations are complete
expect(mockSecrets.store).not.toHaveBeenCalled()
expect(providerSettingsManager.consumeDuplicateIdRepairReport()).toBeNull()
})

it("should dedupe duplicated ids by keeping the first id and assigning new ids to later duplicates", async () => {
mockSecrets.get.mockResolvedValue(
JSON.stringify({
currentApiConfigName: "first",
apiConfigs: {
// insertion order matters here: first keeps id, second changes
first: { id: "dup-id", apiProvider: "anthropic" },
second: { id: "dup-id", apiProvider: "anthropic" },
third: { id: "ok-id", apiProvider: "anthropic" },
},
modeApiConfigs: {},
migrations: {
rateLimitSecondsMigrated: true,
diffSettingsMigrated: true,
openAiHeadersMigrated: true,
consecutiveMistakeLimitMigrated: true,
todoListEnabledMigrated: true,
morphApiKeyMigrated: true,
},
}),
)
// kilocode_change end

await providerSettingsManager.initialize()

expect(mockSecrets.store).toHaveBeenCalled()
const calls = mockSecrets.store.mock.calls
const storedConfig = JSON.parse(calls[calls.length - 1][1])

expect(storedConfig.apiConfigs.first.id).toBe("dup-id")
expect(storedConfig.apiConfigs.second.id).toBeTruthy()
expect(storedConfig.apiConfigs.second.id).not.toBe("dup-id")
expect(storedConfig.apiConfigs.third.id).toBe("ok-id")

const report = providerSettingsManager.consumeDuplicateIdRepairReport()
expect(report).toBeTruthy()
expect(report?.["dup-id"]).toHaveLength(1)
expect(report?.["dup-id"][0]).toBe(storedConfig.apiConfigs.second.id)

// Idempotency: running migrations again on already-deduped config should not write.
const newManager = new ProviderSettingsManager(mockContext)
mockSecrets.store.mockClear()
mockSecrets.get.mockResolvedValueOnce(JSON.stringify(storedConfig))
await newManager.initialize()
expect(mockSecrets.store).not.toHaveBeenCalled()
})

it("should call migrateRateLimitSeconds if it has not done so already", async () => {
mockGlobalState.get.mockResolvedValue(42)

Expand Down