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
2 changes: 1 addition & 1 deletion src/core/config/ProviderSettingsManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export class ProviderSettingsManager {
this.initialize().catch(console.error)
}

private generateId() {
public generateId() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Exposing generateId() as public may allow external misuse. Consider documenting its intended use and, if IDs are security-sensitive, using a more robust method (e.g., crypto.randomUUID()).

return Math.random().toString(36).substring(2, 15)
}

Expand Down
76 changes: 76 additions & 0 deletions src/exports/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,82 @@ export class API extends EventEmitter<RooCodeEvents> implements RooCodeAPI {
await this.sidebarProvider.postStateToWebview()
}

public async createProfile(name: string): Promise<string> {
// Input validation
if (!name || !name.trim()) {
throw new Error("Profile name cannot be empty")
Copy link
Contributor

Choose a reason for hiding this comment

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

Error messages in createProfile are user-facing. Consider internationalizing these strings (e.g., using i18next) to adhere to our standards.

Suggested change
throw new Error("Profile name cannot be empty")
throw new Error(i18next.t('error.profileNameEmpty'))

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure we need to worry about this for the API

Copy link
Contributor

Choose a reason for hiding this comment

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

When checking for existing profiles, an error is thrown using a literal string. Please internationalize this message as well.

Suggested change
throw new Error("Profile name cannot be empty")
throw new Error(i18n.t('error.profileNameEmpty'))

}

const currentSettings = this.getConfiguration()
const profiles = currentSettings.listApiConfigMeta || []

if (profiles.some((profile) => profile.name === name)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not a huge deal, but we might want to improve / expand the ProviderSettingsManager interface and just proxy these calls through to that object. It already has a few things you could use like fetching a profile by name or id. In general the current implementation seems fine though.

throw new Error(`A profile with the name "${name}" already exists`)
}

// Generate unique ID and create profile
const id = this.sidebarProvider.providerSettingsManager.generateId()
const newProfile = {
id,
name: name.trim(),
apiProvider: "openai" as const, // Type assertion for better type safety
}

// Update configuration with new profile
await this.setConfiguration({
...currentSettings,
listApiConfigMeta: [...profiles, newProfile],
})
return id
}

public getProfiles(): string[] {
const profiles = this.getConfiguration().listApiConfigMeta || []
return profiles.map((profile) => profile.name)
}

public async setActiveProfile(name: string): Promise<void> {
const currentSettings = this.getConfiguration()
const profiles = currentSettings.listApiConfigMeta || []

const profile = profiles.find((p) => p.name === name)
if (!profile) {
throw new Error(`Profile with name "${name}" does not exist`)
Copy link
Contributor

Choose a reason for hiding this comment

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

In setActiveProfile, the error message is hard-coded. Consider using internationalized strings for consistency with our development standards.

}

await this.setConfiguration({
...currentSettings,
currentApiConfigName: profile.name,
})
}

public getActiveProfile(): string | undefined {
return this.getConfiguration().currentApiConfigName
}

public async deleteProfile(name: string): Promise<void> {
Copy link
Contributor

Choose a reason for hiding this comment

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

The deleteProfile method throws user-facing errors and resets the active profile without fallback. Consider internationalizing the error and, if appropriate, selecting a different active profile rather than setting it to undefined.

Copy link
Collaborator

@mrubens mrubens Apr 3, 2025

Choose a reason for hiding this comment

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

In the API case is it better to have no active profile after deleting than to switch to another one like we do in the extension?

const currentSettings = this.getConfiguration()
const profiles = currentSettings.listApiConfigMeta || []
const targetIndex = profiles.findIndex((p) => p.name === name)
if (targetIndex === -1) {
throw new Error(`Profile with name "${name}" does not exist`)
}

const profileToDelete = profiles[targetIndex]
profiles.splice(targetIndex, 1)

// If we're deleting the active profile, clear the currentApiConfigName
const newSettings: RooCodeSettings = {
...currentSettings,
listApiConfigMeta: profiles,
currentApiConfigName:
currentSettings.currentApiConfigName === profileToDelete.name
? undefined
: currentSettings.currentApiConfigName,
}
await this.setConfiguration(newSettings)
}

public isReady() {
return this.sidebarProvider.viewLaunched
}
Expand Down
33 changes: 33 additions & 0 deletions src/exports/interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,39 @@ export interface RooCodeAPI extends EventEmitter<RooCodeEvents> {
*/
setConfiguration(values: RooCodeSettings): Promise<void>

/**
* Creates a new API configuration profile
* @param name The name of the profile
* @returns The ID of the created profile
*/
createProfile(name: string): Promise<string>

/**
* Returns a list of all configured profile names
* @returns Array of profile names
*/
getProfiles(): string[]

/**
* Changes the active API configuration profile
* @param name The name of the profile to activate
* @throws Error if the profile does not exist
*/
setActiveProfile(name: string): Promise<void>

/**
* Returns the name of the currently active profile
* @returns The profile name, or undefined if no profile is active
*/
getActiveProfile(): string | undefined

/**
* Deletes a profile by name
* @param name The name of the profile to delete
* @throws Error if the profile does not exist
*/
deleteProfile(name: string): Promise<void>

/**
* Returns true if the API is ready to use.
*/
Expand Down
28 changes: 28 additions & 0 deletions src/exports/roo-code.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,34 @@ interface RooCodeAPI extends EventEmitter<RooCodeEvents> {
* @param values An object containing key-value pairs to set.
*/
setConfiguration(values: RooCodeSettings): Promise<void>
/**
* Creates a new API configuration profile
* @param name The name of the profile
* @returns The ID of the created profile
*/
createProfile(name: string): Promise<string>
/**
* Returns a list of all configured profile names
* @returns Array of profile names
*/
getProfiles(): string[]
/**
* Changes the active API configuration profile
* @param name The name of the profile to activate
* @throws Error if the profile does not exist
*/
setActiveProfile(name: string): Promise<void>
/**
* Returns the name of the currently active profile
* @returns The profile name, or undefined if no profile is active
*/
getActiveProfile(): string | undefined
/**
* Deletes a profile by name
* @param name The name of the profile to delete
* @throws Error if the profile does not exist
*/
deleteProfile(name: string): Promise<void>
/**
* Returns true if the API is ready to use.
*/
Expand Down