-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Test a provider-oriented welcome screen #9484
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 |
|---|---|---|
|
|
@@ -252,8 +252,9 @@ export class WebAuthService extends EventEmitter<AuthServiceEvents> implements A | |
| * and opening the browser to the authorization URL. | ||
| * | ||
| * @param landingPageSlug Optional slug of a specific landing page (e.g., "supernova", "special-offer", etc.) | ||
| * @param useProviderSignup If true, uses provider signup flow (/extension/provider-sign-up). If false, uses standard sign-in (/extension/sign-in). Defaults to false. | ||
| */ | ||
| public async login(landingPageSlug?: string): Promise<void> { | ||
| public async login(landingPageSlug?: string, useProviderSignup: boolean = false): Promise<void> { | ||
| try { | ||
| const vscode = await importVscode() | ||
|
|
||
|
|
@@ -272,10 +273,12 @@ export class WebAuthService extends EventEmitter<AuthServiceEvents> implements A | |
| auth_redirect: `${vscode.env.uriScheme}://${publisher}.${name}`, | ||
| }) | ||
|
|
||
| // Use landing page URL if slug is provided, otherwise use default sign-in URL | ||
| // Use landing page URL if slug is provided, otherwise use provider sign-up or sign-in URL based on parameter | ||
| const url = landingPageSlug | ||
| ? `${getRooCodeApiUrl()}/l/${landingPageSlug}?${params.toString()}` | ||
| : `${getRooCodeApiUrl()}/extension/sign-in?${params.toString()}` | ||
| : useProviderSignup | ||
| ? `${getRooCodeApiUrl()}/extension/provider-sign-up?${params.toString()}` | ||
| : `${getRooCodeApiUrl()}/extension/sign-in?${params.toString()}` | ||
|
|
||
| await vscode.env.openExternal(vscode.Uri.parse(url)) | ||
| } catch (error) { | ||
|
|
@@ -294,11 +297,13 @@ export class WebAuthService extends EventEmitter<AuthServiceEvents> implements A | |
| * @param code The authorization code from the callback | ||
| * @param state The state parameter from the callback | ||
| * @param organizationId The organization ID from the callback (null for personal accounts) | ||
| * @param providerModel The model ID selected during signup (optional) | ||
| */ | ||
| public async handleCallback( | ||
| code: string | null, | ||
| state: string | null, | ||
| organizationId?: string | null, | ||
| providerModel?: string | null, | ||
| ): Promise<void> { | ||
| if (!code || !state) { | ||
| const vscode = await importVscode() | ||
|
|
@@ -326,6 +331,12 @@ export class WebAuthService extends EventEmitter<AuthServiceEvents> implements A | |
|
|
||
| await this.storeCredentials(credentials) | ||
|
|
||
| // Store the provider model if provided | ||
| if (providerModel) { | ||
| await this.context.globalState.update("roo-provider-model", providerModel) | ||
| this.log(`[auth] Stored provider model: ${providerModel}`) | ||
| } | ||
|
Comment on lines
+334
to
+338
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. Missing error cleanup: If authentication fails after storing the provider model but before credential storage completes, the stored model persists in global state. On the next successful login, this orphaned model could be unexpectedly applied. The stored model should be cleaned up in the catch block to prevent this scenario. Fix it with Roo Code or mention @roomote and request a fix. |
||
|
|
||
| const vscode = await importVscode() | ||
|
|
||
| if (vscode) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -168,6 +168,31 @@ export async function activate(context: vscode.ExtensionContext) { | |
|
|
||
| if (data.state === "active-session" || data.state === "logged-out") { | ||
| await handleRooModelsCache() | ||
|
|
||
| // Apply stored provider model to API configuration if present | ||
| if (data.state === "active-session") { | ||
| try { | ||
| const storedModel = context.globalState.get<string>("roo-provider-model") | ||
| if (storedModel) { | ||
| cloudLogger(`[authStateChangedHandler] Applying stored provider model: ${storedModel}`) | ||
| // Get the current API configuration name | ||
| const currentConfigName = | ||
| provider.contextProxy.getGlobalState("currentApiConfigName") || "default" | ||
| // Update it with the stored model using upsertProviderProfile | ||
| await provider.upsertProviderProfile(currentConfigName, { | ||
| apiProvider: "roo", | ||
| apiModelId: storedModel, | ||
| }) | ||
| // Clear the stored model after applying | ||
| await context.globalState.update("roo-provider-model", undefined) | ||
| cloudLogger(`[authStateChangedHandler] Applied and cleared stored provider model`) | ||
| } | ||
|
Comment on lines
+173
to
+189
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. Race condition: The stored provider model is applied asynchronously during auth state change, but there's no guarantee it completes before the user dismisses the welcome screen or makes other configuration changes. If authentication completes quickly, the model could be applied after the user has already selected a different model, overwriting their choice. Consider using a promise or flag to ensure the model application completes before allowing further configuration changes. Fix it with Roo Code or mention @roomote and request a fix. |
||
| } catch (error) { | ||
| cloudLogger( | ||
| `[authStateChangedHandler] Failed to apply stored provider model: ${error instanceof Error ? error.message : String(error)}`, | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
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.
Missing validation: The
providerModelparameter is stored without validation. If an invalid model ID is passed (malformed format, non-existent model, or incompatible with the Roo provider), it will be stored and later applied inextension.tswithout checks, potentially breaking the API configuration. Consider validating the model ID format and/or existence before storing.Fix it with Roo Code or mention @roomote and request a fix.