-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Kilo Unified User - Multi Profile Support #4347
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: f9093ae The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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.
✅ No Issues Found
19 files reviewed | Confidence: 92% | Recommendation: Merge
Review Details
Files Reviewed:
packages/types/src/kilocode/kilo-user.ts(new)packages/types/src/index.ts(exports)src/core/kilocode/kilo-user-resolver.ts(new)src/core/kilocode/webview/kiloUserHandler.ts(new)src/core/kilocode/__tests__/kilo-user-resolver.spec.ts(new)webview-ui/src/components/kilocode/common/OrganizationSelector.tsx(modified)webview-ui/src/utils/kilocode/useKiloIdentity.tsx(simplified)src/core/webview/ClineProvider.ts(modified)src/core/webview/webviewMessageHandler.ts(refactored)src/extension.ts(updated import)webview-ui/src/App.tsx(modified)src/shared/ExtensionMessage.ts(added kiloUser to state)src/shared/WebviewMessage.ts(added profileName)
Architecture Assessment:
- ✅ Clean separation of concerns with centralized user resolution
- ✅ Proper priority-based resolution (active profile → first kilocode profile → unauthenticated)
- ✅ Profile filtering in OrganizationSelector prevents cross-profile data contamination
- ✅ Comprehensive test coverage for resolver functions
- ✅ Backward compatible - existing functionality preserved
Security Check:
- ✅ Token handling remains secure (tokens not exposed in KiloUser interface)
- ✅ Profile context properly scoped to prevent auth override bugs
- ✅ No sensitive data leakage in new types
Code Quality:
- ✅ Well-documented with clear JSDoc comments
- ✅ Proper TypeScript typing throughout
- ✅ Error handling with fallback to EMPTY_KILO_USER
- ✅ useCallback with proper dependencies in OrganizationSelector
Checked: Security, bugs, performance, error handling, type safety
Summary
This PR successfully addresses the three issues outlined:
-
Authentication Override Bug - Fixed by adding
profileNameparameter to requests and only performing organization validation for the active profile -
Single Profile Limitation - OrganizationSelector now supports controlled mode with
apiConfiguration,profileName, andonChangeprops -
Scattered User Resolution Logic - Consolidated into
kilo-user-resolver.tswith three clean exported functions
The refactoring is well-executed with proper test coverage. The code follows established patterns in the codebase and maintains backward compatibility.
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.
✅ No New Issues
Incremental review of commit f9093ae (test fix since previous review).
The new commit properly updates the test mocks in webviewMessageHandler.autoSwitch.spec.ts to align with the refactored code that now uses resolveGlobalProfile. Changes include:
- Added
listConfigmock tomockProviderSettingsManager(required by the new profile resolution logic) - Updated
getProfilemock to return proper kilocode profile structure withapiProvider,kilocodeToken, andkilocodeOrganizationId - Added specific mock for the "already has organization selected" test case to properly simulate existing organization state
These test fixes are correct and necessary for the refactored architecture.
Review Details
Files Changed Since Last Review:
src/core/webview/__tests__/webviewMessageHandler.autoSwitch.spec.ts(test fixes)
Verified:
- ✅ Test mocks correctly match the new
resolveGlobalProfilefunction signature - ✅ Mock data structure aligns with
ProfileContextinterface - ✅ Test assertions remain valid for the refactored behavior
Context
This PR refactors the Kilocode authentication and user resolution system to support multiple Kilocode provider profiles seamlessly. Previously, the system had several critical issues:
Authentication Override Bug: When users authenticated via the
vscode://URL handler (device auth flow), the system would override ALL Kilocode provider profiles with the new authentication token, losing existing profile configurations.Single Profile Limitation: The organization selector and API requests only worked with the currently active profile, making it impossible to view or manage organizations for other Kilocode profiles without switching the active profile first.
Scattered User Resolution Logic: User authentication and token resolution logic was duplicated across multiple files, making it difficult to maintain consistent behavior.
Implementation
Core Changes
Centralized User Resolution (
src/core/kilocode/kilo-user-resolver.ts)resolveKiloUserProfile(): Returns full profile contextresolveKiloUserToken(): Returns just the tokenresolveKiloUser(): Returns user identity for telemetryDedicated Kilo User Handler (
src/core/kilocode/webview/kiloUserHandler.ts)Enhanced Organization Selector (
webview-ui/src/components/kilocode/common/OrganizationSelector.tsx)Type-Safe User Identity (
packages/types/src/kilocode/kilo-user.ts)KiloUserinterface for consistent user representationBug Fixes
Authentication Override Prevention:
Multi-Profile Support:
profileNameparameterCode Organization:
webviewMessageHandler.tsScreenshots
How to Test
Test Multi-Profile Authentication
Test Organization Selector
Test Global User Resolution