-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Add profile management functions to API #2251
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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") | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Error messages in
Suggested change
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure we need to worry about this for the API
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||||||||||
| } | ||||||||||
|
|
||||||||||
| const currentSettings = this.getConfiguration() | ||||||||||
| const profiles = currentSettings.listApiConfigMeta || [] | ||||||||||
|
|
||||||||||
| if (profiles.some((profile) => profile.name === name)) { | ||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a huge deal, but we might want to improve / expand the |
||||||||||
| 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`) | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In |
||||||||||
| } | ||||||||||
|
|
||||||||||
| await this.setConfiguration({ | ||||||||||
| ...currentSettings, | ||||||||||
| currentApiConfigName: profile.name, | ||||||||||
| }) | ||||||||||
| } | ||||||||||
|
|
||||||||||
| public getActiveProfile(): string | undefined { | ||||||||||
| return this.getConfiguration().currentApiConfigName | ||||||||||
| } | ||||||||||
|
|
||||||||||
| public async deleteProfile(name: string): Promise<void> { | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||||||||||
| } | ||||||||||
|
|
||||||||||
There was a problem hiding this comment.
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()).