Skip to content
Open
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
57 changes: 53 additions & 4 deletions src/__tests__/ClineProvider.delegation.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,8 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => {
isViewLaunched: false,
taskHistoryStore: makeStoreStub(),
providerSettingsManager,
// No window-local choice is recorded, so the lookup reaches the shared manager.
getModeConfigId: (mode: string) => providerSettingsManager.getModeConfigId(mode),
Comment thread
PierrunoYT marked this conversation as resolved.
Resolved
context: { workspaceState: { get: workspaceGet } },
} as unknown as ClineProvider

Expand Down Expand Up @@ -325,6 +327,53 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => {
)
})

it("uses this window's mode profile instead of one another window saved in shared storage", async () => {
const parentTask = makeParentTask()
const createTask = vi
.fn()
.mockResolvedValue({ taskId: "child-local", run: vi.fn().mockResolvedValue(undefined) })
const getProfile = vi.fn().mockResolvedValue({
name: "local-profile",
id: "local-profile-id",
apiProvider: providerIdentifiers.openrouter,
openRouterModelId: "openai/gpt-4.1-mini",
})
const sharedGetModeConfigId = vi.fn().mockResolvedValue("other-window-profile-id")
const provider = {
taskScheduler: new TaskScheduler(),
emit: vi.fn(),
getCurrentTask: vi.fn(() => parentTask),
removeClineFromStack: vi.fn().mockResolvedValue(undefined),
createTask,
log: vi.fn(),
isViewLaunched: false,
taskHistoryStore: makeStoreStub(),
providerSettingsManager: { getModeConfigId: sharedGetModeConfigId, getProfile },
// This window's own choice differs from the shared value another window wrote.
getModeConfigId: vi.fn().mockResolvedValue("local-profile-id"),
context: { workspaceState: { get: vi.fn().mockReturnValue(false) } },
} as unknown as ClineProvider

await ClineProvider.prototype.delegateParentAndOpenChild.call(provider, {
parentTaskId: "parent-1",
message: "Local child",
initialTodos: [],
mode: "ask",
})

expect(getProfile).toHaveBeenCalledOnce()
expect(getProfile).toHaveBeenCalledWith({ id: "local-profile-id" })
expect(sharedGetModeConfigId).not.toHaveBeenCalled()
expect(createTask).toHaveBeenCalledWith(
"Local child",
undefined,
parentTask,
expect.objectContaining({
handoffExecutionContext: expect.objectContaining({ mode: "ask", apiConfigName: "local-profile" }),
}),
)
})

it.each([
{ name: "has no saved mode profile", savedConfigId: undefined, savedProfile: undefined },
{
Expand All @@ -346,6 +395,7 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => {
? vi.fn().mockRejectedValue(savedProfile)
: vi.fn().mockResolvedValue(savedProfile)
const log = vi.fn()
const getModeConfigId = vi.fn().mockResolvedValue(savedConfigId)
const provider = {
taskScheduler: new TaskScheduler(),
emit: vi.fn(),
Expand All @@ -355,10 +405,9 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => {
log,
isViewLaunched: false,
taskHistoryStore: makeStoreStub(),
providerSettingsManager: {
getModeConfigId: vi.fn().mockResolvedValue(savedConfigId),
getProfile,
},
providerSettingsManager: { getModeConfigId, getProfile },
// No window-local choice is recorded, so the lookup reaches the shared manager.
getModeConfigId,
context: { workspaceState: { get: vi.fn().mockReturnValue(false) } },
} as unknown as ClineProvider

Expand Down
26 changes: 23 additions & 3 deletions src/core/config/ProviderSettingsManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ export class ProviderSettingsManager {
/**
* Set the API config for a specific mode.
*/
public async setModeConfig(mode: Mode, configId: string) {
public async setModeConfig(mode: Mode, configId: string, onPersisted?: () => void) {
try {
return await this.lock(async () => {
const providerProfiles = await this.load()
Expand All @@ -554,6 +554,7 @@ export class ProviderSettingsManager {
// Assign the chosen config ID to this mode
providerProfiles.modeApiConfigs[mode] = configId
await this.store(providerProfiles)
onPersisted?.()
})
} catch (error) {
throw new Error(`Failed to set mode config: ${error}`)
Expand All @@ -574,6 +575,21 @@ export class ProviderSettingsManager {
}
}

/**
* Get a copy of the persisted mode -> API config ID map. The map is shared by every VS Code
* window, so callers that need a stable view must keep their own copy.
*/
public async getModeConfigIds(): Promise<Record<string, string>> {
try {
return await this.lock(async () => {
const { modeApiConfigs } = await this.load()
return { ...modeApiConfigs }
})
} catch (error) {
throw new Error(`Failed to get mode configs: ${error}`)
}
}

public async export() {
try {
return await this.lock(async () => {
Expand Down Expand Up @@ -636,9 +652,13 @@ export class ProviderSettingsManager {
}
}

public async import(providerProfiles: ProviderProfiles) {
public async import(providerProfiles: ProviderProfiles, onPersisted?: () => void) {
try {
return await this.lock(() => this.store(providerProfiles))
return await this.lock(async () => {
await this.store(providerProfiles)
// Publish local choices before a later writer can acquire the storage lock.
onPersisted?.()
})
} catch (error) {
throw new Error(`Failed to import provider profiles: ${error}`)
}
Expand Down
65 changes: 65 additions & 0 deletions src/core/config/__tests__/ProviderSettingsManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,36 @@ describe("ProviderSettingsManager", () => {
providerSettingsManager = new ProviderSettingsManager(mockContext)
})

it.each([
{ operation: "import", fails: false },
{ operation: "import", fails: true },
{ operation: "mode write", fails: false },
{ operation: "mode write", fails: true },
])("publishes $operation choices only after persistence (failure: $fails)", async ({ operation, fails }) => {
let release = () => {}
const pending = new Promise<void>((resolve) => {
release = resolve
})
mockSecrets.store.mockImplementationOnce(async () => {
await pending
if (fails) throw new Error("write failed")
})
const published = vi.fn()
const write =
operation === "import"
? providerSettingsManager.import(
{ currentApiConfigName: "default", apiConfigs: {}, modeApiConfigs: { code: "imported-id" } },
published,
)
: providerSettingsManager.setModeConfig("code", "chosen-id", published)
await vi.waitFor(() => expect(mockSecrets.store).toHaveBeenCalledOnce())
expect(published).not.toHaveBeenCalled()
const result = fails ? expect(write).rejects.toThrow("write failed") : expect(write).resolves.toBeUndefined()
release()
await result
expect(published).toHaveBeenCalledTimes(fails ? 0 : 1)
})

describe("initialize", () => {
it("should not write to storage when secrets.get returns null", async () => {
// Mock readConfig to return null
Expand Down Expand Up @@ -1352,6 +1382,41 @@ describe("ProviderSettingsManager", () => {
})
})

describe("GetModeConfigIds", () => {
const profilesWithModes = (modeApiConfigs?: Record<string, string>) =>
JSON.stringify({
currentApiConfigName: "default",
apiConfigs: { default: { id: "default" } },
modeApiConfigs,
migrations: { rateLimitSecondsMigrated: true },
})

it("returns a copy of the stored mode map", async () => {
mockSecrets.get.mockResolvedValue(profilesWithModes({ code: "code-id", architect: "architect-id" }))

const ids = await providerSettingsManager.getModeConfigIds()
ids.code = "changed"

expect(ids).toEqual({ code: "changed", architect: "architect-id" })
await expect(providerSettingsManager.getModeConfigIds()).resolves.toEqual({
code: "code-id",
architect: "architect-id",
})
})

it("returns an empty map when none is stored", async () => {
mockSecrets.get.mockResolvedValue(profilesWithModes(undefined))

await expect(providerSettingsManager.getModeConfigIds()).resolves.toEqual({})
})

it("should throw error if secrets storage fails", async () => {
mockSecrets.get.mockRejectedValue(new Error("Storage failed"))

await expect(providerSettingsManager.getModeConfigIds()).rejects.toThrow("Failed to get mode configs")
})
})

describe("HasConfig", () => {
it("should return true for existing config", async () => {
const existingConfig: ProviderProfiles = {
Expand Down
32 changes: 31 additions & 1 deletion src/core/config/__tests__/importExport.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,13 +187,38 @@ describe("importExport", () => {
beforeEach(() => {
;(fs.readFile as Mock).mockResolvedValue(validFileContent)
mockProviderSettingsManager.export.mockResolvedValue(previousProviderProfiles)
mockProviderSettingsManager.import.mockResolvedValue(undefined)
mockProviderSettingsManager.import.mockImplementation(async (_profiles, onPersisted) => {
onPersisted?.()
})
mockProviderSettingsManager.listConfig.mockResolvedValue(profileMetadata)
mockContextProxy.setValues.mockResolvedValue(undefined)
mockContextProxy.setValue.mockResolvedValue(undefined)
mockContextProxy.setProviderSettings.mockResolvedValue(undefined)
})

it.each([false, true])(
"updates the importing window only after successful persistence (failure: %s)",
async (fails) => {
const modeApiConfigs = { "new-custom-mode": "test-id" }
const file = JSON.parse(validFileContent)
file.providerProfiles.modeApiConfigs = modeApiConfigs
vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(file))
const onModeConfigIdsImported = vi.fn()
if (fails) mockProviderSettingsManager.import.mockRejectedValueOnce(new Error("write failed"))

const result = await importSettingsFromPath("/mock/path/settings.json", {
providerSettingsManager: mockProviderSettingsManager,
contextProxy: mockContextProxy,
customModesManager: mockCustomModesManager,
onModeConfigIdsImported,
})

expect(result.success).toBe(!fails)
if (fails) expect(onModeConfigIdsImported).not.toHaveBeenCalled()
else expect(onModeConfigIdsImported).toHaveBeenCalledExactlyOnceWith(modeApiConfigs)
},
)

it("waits for each provider-state write before reporting success", async () => {
const currentProfileWrite = createDeferred<void>()
const providerSettingsWrite = createDeferred<void>()
Expand Down Expand Up @@ -638,6 +663,7 @@ describe("importExport", () => {
// Create a mock provider for the test
const mockProvider = {
settingsImportedAt: 0,
adoptModeConfigIds: vi.fn(),
postStateToWebview: vi.fn().mockResolvedValue(undefined),
postStateToWebviewWithoutTaskHistory: vi.fn().mockResolvedValue(undefined),
}
Expand Down Expand Up @@ -921,6 +947,7 @@ describe("importExport", () => {
const seenImportedAt: Array<number | undefined> = []
const mockProvider = {
settingsImportedAt: undefined as number | undefined,
adoptModeConfigIds: vi.fn(),
postStateToWebview: vi.fn().mockImplementation(async () => {
seenImportedAt.push(mockProvider.settingsImportedAt)
}),
Expand Down Expand Up @@ -997,6 +1024,7 @@ describe("importExport", () => {
const seenImportedAt: Array<number | undefined> = []
const mockProvider = {
settingsImportedAt: undefined as number | undefined,
adoptModeConfigIds: vi.fn(),
postStateToWebview: vi.fn().mockImplementation(async () => {
seenImportedAt.push(mockProvider.settingsImportedAt)
}),
Expand Down Expand Up @@ -1268,6 +1296,7 @@ describe("importExport", () => {

const mockProvider = {
settingsImportedAt: 0,
adoptModeConfigIds: vi.fn(),
postStateToWebview: vi.fn().mockResolvedValue(undefined),
}

Expand Down Expand Up @@ -1537,6 +1566,7 @@ describe("importExport", () => {

const mockProvider = {
settingsImportedAt: 0,
adoptModeConfigIds: vi.fn(),
postStateToWebview: vi.fn().mockResolvedValue(undefined),
}

Expand Down
31 changes: 26 additions & 5 deletions src/core/config/importExport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
providerSettingsManager: ProviderSettingsManager
contextProxy: ContextProxy
customModesManager: CustomModesManager
onModeConfigIdsImported?: (ids: Record<string, string>) => void
}

type ExportOptions = {
Expand All @@ -37,6 +38,7 @@
provider: {
settingsImportedAt?: number
postStateToWebview: () => Promise<void>
adoptModeConfigIds: (ids: Record<string, string>) => void
}
}

Expand Down Expand Up @@ -135,7 +137,7 @@
*/
export async function importSettingsFromPath(
filePath: string,
{ providerSettingsManager, contextProxy, customModesManager }: ImportOptions,
{ providerSettingsManager, contextProxy, customModesManager, onModeConfigIdsImported }: ImportOptions,
) {
// Use a lenient schema that accepts any apiConfigs, then validate each individually
const lenientProviderProfilesSchema = providerProfilesSchema.extend({
Expand Down Expand Up @@ -227,7 +229,13 @@
// OpenAI Compatible settings are now correctly stored in codebaseIndexConfig
// They will be imported automatically with the config - no special handling needed

await providerSettingsManager.import(providerProfiles)
if (onModeConfigIdsImported) {
await providerSettingsManager.import(providerProfiles, () =>
onModeConfigIdsImported(rawProviderProfiles.modeApiConfigs ?? {}),
)
} else {
await providerSettingsManager.import(providerProfiles)
}
await contextProxy.setValues(sanitizedGlobalSettings)

// Set the current provider.
Expand Down Expand Up @@ -269,7 +277,12 @@
* @param options - Import options containing managers and proxy
* @returns Promise resolving to import result
*/
export const importSettings = async ({ providerSettingsManager, contextProxy, customModesManager }: ImportOptions) => {
export const importSettings = async ({
providerSettingsManager,
contextProxy,
customModesManager,
onModeConfigIdsImported,
}: ImportOptions) => {
// Use the last export path as a sensible default, falling back to Downloads
const defaultUri = resolveDefaultSaveUri(contextProxy, "lastSettingsExportPath", "zoo-code-settings.json", {
useWorkspace: false,
Expand All @@ -290,6 +303,7 @@
providerSettingsManager,
contextProxy,
customModesManager,
onModeConfigIdsImported,
})
}

Expand All @@ -302,13 +316,14 @@
* @returns Promise resolving to import result
*/
export const importSettingsFromFile = async (
{ providerSettingsManager, contextProxy, customModesManager }: ImportOptions,
{ providerSettingsManager, contextProxy, customModesManager, onModeConfigIdsImported }: ImportOptions,
fileUri: vscode.Uri,
) => {
return importSettingsFromPath(fileUri.fsPath, {
providerSettingsManager,
contextProxy,
customModesManager,
onModeConfigIdsImported,
})
}

Expand Down Expand Up @@ -374,6 +389,7 @@
providerSettingsManager,
contextProxy,
customModesManager,
onModeConfigIdsImported: (ids) => provider.adoptModeConfigIds(ids),

Check warning on line 392 in src/core/config/importExport.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory

src/core/config/importExport.ts:392: Survived ArrowFunction mutant (replacement: () => undefined). See the job summary for the complete list and resolution guidance.
})
} catch (error) {
result = {
Expand All @@ -382,7 +398,12 @@
}
}
} else {
result = await importSettings({ providerSettingsManager, contextProxy, customModesManager })
result = await importSettings({
providerSettingsManager,
contextProxy,
customModesManager,
onModeConfigIdsImported: (ids) => provider.adoptModeConfigIds(ids),
})
}

if (result.success) {
Expand Down
Loading
Loading