Skip to content

Commit

Permalink
ConfigHandler: make updateIdeSettings work
Browse files Browse the repository at this point in the history
ConfigHandler.updateIdeSettings() was largely ineffective because the
ideSettingsPromise was stored in the ProfileLoader object, so updating
to a new ideSettingsPromise resulted in the config being reloaded
with the old settings. Fix this by passing the ideSettingsPromise as
a parameter to IProfileLoader.doLoadConfig().
  • Loading branch information
owtaylor committed Dec 19, 2024
1 parent 85f27b9 commit 9767cee
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 16 deletions.
7 changes: 3 additions & 4 deletions core/config/ConfigHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ export class ConfigHandler {
// Set local profile as default
const localProfileLoader = new LocalProfileLoader(
ide,
ideSettingsPromise,
controlPlaneClient,
writeLog,
);
Expand Down Expand Up @@ -111,7 +110,6 @@ export class ConfigHandler {
workspace.name,
this.controlPlaneClient,
this.ide,
this.ideSettingsPromise,
this.writeLog,
this.reloadConfig.bind(this),
);
Expand Down Expand Up @@ -214,7 +212,7 @@ export class ConfigHandler {
// TODO: this isn't right, there are two different senses in which you want to "reload"

const { config, errors, configLoadInterrupted } =
await this.currentProfile.reloadConfig();
await this.currentProfile.reloadConfig(this.ideSettingsPromise);

if (config) {
this.inactiveProfiles.forEach((profile) => profile.clearConfig());
Expand All @@ -226,6 +224,7 @@ export class ConfigHandler {

getSerializedConfig(): Promise<BrowserSerializedContinueConfig> {
return this.currentProfile.getSerializedConfig(
this.ideSettingsPromise,
this.additionalContextProviders,
);
}
Expand All @@ -235,7 +234,7 @@ export class ConfigHandler {
}

async loadConfig(): Promise<ContinueConfig> {
return this.currentProfile.loadConfig(this.additionalContextProviders);
return this.currentProfile.loadConfig(this.ideSettingsPromise, this.additionalContextProviders);
}

async llmFromTitle(title?: string): Promise<ILLM> {
Expand Down
11 changes: 7 additions & 4 deletions core/config/ProfileLifecycleManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
BrowserSerializedContinueConfig,
ContinueConfig,
IContextProvider,
IdeSettings,
} from "../index.js";

import { ConfigResult, finalToBrowserConfig } from "./load.js";
Expand Down Expand Up @@ -41,15 +42,16 @@ export class ProfileLifecycleManager {
}

// Clear saved config and reload
async reloadConfig(): Promise<ConfigResult<ContinueConfig>> {
async reloadConfig(ideSettingsPromise: Promise<IdeSettings>): Promise<ConfigResult<ContinueConfig>> {
this.savedConfig = undefined;
this.savedBrowserConfig = undefined;
this.pendingConfigPromise = undefined;

return this.profileLoader.doLoadConfig();
return this.profileLoader.doLoadConfig(ideSettingsPromise);
}

async loadConfig(
ideSettingsPromise: Promise<IdeSettings>,
additionalContextProviders: IContextProvider[],
): Promise<ContinueConfig> {
// If we already have a config, return it
Expand All @@ -62,7 +64,7 @@ export class ProfileLifecycleManager {
// Set pending config promise
this.pendingConfigPromise = new Promise(async (resolve, reject) => {
const { config: newConfig, errors } =
await this.profileLoader.doLoadConfig();
await this.profileLoader.doLoadConfig(ideSettingsPromise);

if (newConfig) {
// Add registered context providers
Expand All @@ -86,10 +88,11 @@ export class ProfileLifecycleManager {
}

async getSerializedConfig(
ideSettingsPromise: Promise<IdeSettings>,
additionalContextProviders: IContextProvider[],
): Promise<BrowserSerializedContinueConfig> {
if (!this.savedBrowserConfig) {
const continueConfig = await this.loadConfig(additionalContextProviders);
const continueConfig = await this.loadConfig(ideSettingsPromise, additionalContextProviders);
this.savedBrowserConfig = finalToBrowserConfig(continueConfig);
}
return this.savedBrowserConfig;
Expand Down
5 changes: 2 additions & 3 deletions core/config/profile/ControlPlaneProfileLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ export default class ControlPlaneProfileLoader implements IProfileLoader {
private workspaceTitle: string,
private readonly controlPlaneClient: ControlPlaneClient,
private readonly ide: IDE,
private ideSettingsPromise: Promise<IdeSettings>,
private writeLog: (message: string) => Promise<void>,
private readonly onReload: () => void,
) {
Expand All @@ -39,7 +38,7 @@ export default class ControlPlaneProfileLoader implements IProfileLoader {
}, ControlPlaneProfileLoader.RELOAD_INTERVAL);
}

async doLoadConfig(): Promise<ConfigResult<ContinueConfig>> {
async doLoadConfig(ideSettingsPromise: Promise<IdeSettings>): Promise<ConfigResult<ContinueConfig>> {
const settings =
this.workspaceSettings ??
((await this.controlPlaneClient.getSettingsForWorkspace(
Expand All @@ -49,7 +48,7 @@ export default class ControlPlaneProfileLoader implements IProfileLoader {

const results = await doLoadConfig(
this.ide,
this.ideSettingsPromise,
ideSettingsPromise,
this.controlPlaneClient,
this.writeLog,
serializedConfig,
Expand Down
7 changes: 5 additions & 2 deletions core/config/profile/IProfileLoader.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
// ProfileHandlers manage the loading of a config, allowing us to abstract over different ways of getting to a ContinueConfig

import { ContinueConfig } from "../../index.js";
import {
ContinueConfig,
IdeSettings,
} from "../../index.js";
import { ConfigResult } from "../load.js";

// After we have the ContinueConfig, the ConfigHandler takes care of everything else (loading models, lifecycle, etc.)
export interface IProfileLoader {
profileTitle: string;
profileId: string;
doLoadConfig(): Promise<ConfigResult<ContinueConfig>>;
doLoadConfig(ideSettingsPromise: Promise<IdeSettings>): Promise<ConfigResult<ContinueConfig>>;
setIsActive(isActive: boolean): void;
}
5 changes: 2 additions & 3 deletions core/config/profile/LocalProfileLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,14 @@ export default class LocalProfileLoader implements IProfileLoader {

constructor(
private ide: IDE,
private ideSettingsPromise: Promise<IdeSettings>,
private controlPlaneClient: ControlPlaneClient,
private writeLog: (message: string) => Promise<void>,
) {}

async doLoadConfig(): Promise<ConfigResult<ContinueConfig>> {
async doLoadConfig(ideSettingsPromise: Promise<IdeSettings>): Promise<ConfigResult<ContinueConfig>> {
return doLoadConfig(
this.ide,
this.ideSettingsPromise,
ideSettingsPromise,
this.controlPlaneClient,
this.writeLog,
undefined,
Expand Down

0 comments on commit 9767cee

Please sign in to comment.