-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Fix: Add 'roo' provider to checkExistKey function #7239
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
Conversation
- Added 'roo' to the special case list in checkExistApiConfig.ts - This allows the welcome screen to dismiss properly for Roo Code Cloud provider - Roo provider uses cloud authentication instead of API keys, so it doesn't need configuration
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.
Thank you for your contribution! The change correctly adds 'roo' to the special case list, which aligns with how the RooHandler uses CloudService session tokens for authentication instead of API keys. This should resolve the 'Let's go' button issue for the Roo Code Cloud provider. I've left some suggestions inline that could improve the implementation.
| // Special case for human-relay, fake-ai, and claude-code providers which don't need any configuration. | ||
| if (config.apiProvider && ["human-relay", "fake-ai", "claude-code"].includes(config.apiProvider)) { | ||
| // Special case for human-relay, fake-ai, claude-code, and roo providers which don't need any configuration. | ||
| if (config.apiProvider && ["human-relay", "fake-ai", "claude-code", "roo"].includes(config.apiProvider)) { |
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.
Would it be helpful to add test coverage for these special case providers? I noticed the test file doesn't include any tests verifying that 'roo' (and the other special providers) return true without requiring configuration.
| // Special case for human-relay, fake-ai, and claude-code providers which don't need any configuration. | ||
| if (config.apiProvider && ["human-relay", "fake-ai", "claude-code"].includes(config.apiProvider)) { | ||
| // Special case for human-relay, fake-ai, claude-code, and roo providers which don't need any configuration. | ||
| if (config.apiProvider && ["human-relay", "fake-ai", "claude-code", "roo"].includes(config.apiProvider)) { |
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.
Consider extracting this array as a constant like NO_CONFIG_REQUIRED_PROVIDERS to improve maintainability. This would make it easier to find all providers that don't require configuration and could be reused if needed elsewhere:
| if (config.apiProvider && ["human-relay", "fake-ai", "claude-code", "roo"].includes(config.apiProvider)) { | |
| const NO_CONFIG_REQUIRED_PROVIDERS = ["human-relay", "fake-ai", "claude-code", "roo"] | |
| // Special case for providers that use alternative authentication methods instead of API keys. | |
| if (config.apiProvider && NO_CONFIG_REQUIRED_PROVIDERS.includes(config.apiProvider)) { |
|
|
||
| // Special case for human-relay, fake-ai, and claude-code providers which don't need any configuration. | ||
| if (config.apiProvider && ["human-relay", "fake-ai", "claude-code"].includes(config.apiProvider)) { | ||
| // Special case for human-relay, fake-ai, claude-code, and roo providers which don't need any configuration. |
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.
The comment could be more specific about why these providers are special. Perhaps something like: "Special case for providers that use alternative authentication methods (session tokens, relay, etc.) instead of API keys"?
Fixes the 'Let's go' button not working for Roo Code Cloud provider on the welcome screen.
Added 'roo' to the special case list in checkExistApiConfig.ts since it uses cloud authentication instead of API keys.
Important
Adds 'roo' to special case list in
checkExistApiConfig.tsto fix 'Let's go' button for Roo Code Cloud provider.checkExistApiConfig.ts.This description was created by
for a1a0f44. You can customize this summary. It will automatically update as commits are pushed.