[cli] feat: implement profile management UI with list, detail, editor, and direct command access #1070
Conversation
WalkthroughAdds interactive profile management: ProfileList, ProfileDetail, and ProfileEditor dialogs; a new useProfileManagement hook; slash commands (show, edit, updated list); runtime API getProfileByName; and wiring across AppContainer, DialogManager, contexts, reducers, and tests to support list/view/edit flows. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SlashCmd as Slash Command Processor
participant App as AppContainer
participant Hook as useProfileManagement
participant Manager as ProfileManager
participant Dialog as Dialog Components
User->>SlashCmd: /profile list
SlashCmd->>App: openProfileListDialog()
App->>Hook: request profiles
Hook->>Manager: listSavedProfiles()
Manager-->>Hook: profiles[]
Hook->>App: update UIState (profileListItems)
App->>Dialog: render ProfileListDialog
Dialog-->>User: show profiles
User->>Dialog: select profile -> view detail
Dialog->>App: viewProfileDetail(name)
App->>Hook: loadProfile(name)
Hook->>Manager: loadProfile(name)
Manager-->>Hook: Profile
Hook->>App: set selectedProfileData
App->>Dialog: render ProfileDetailDialog
User->>Dialog: edit -> open editor
Dialog->>App: openProfileEditor(name)
App->>Dialog: render ProfileInlineEditor
User->>Dialog: save (Ctrl+S)
Dialog->>App: saveProfileFromEditor(name, updated)
App->>Hook: saveProfile(updated)
Hook->>Manager: saveProfile(updated)
Manager-->>Hook: success
Hook->>App: refresh list & emit message
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @packages/cli/src/ui/components/ProfileInlineEditor.tsx:
- Around line 113-124: The code currently parses JSON and casts it to Profile
without schema checks; before calling onSave(profileName, updatedProfile)
validate the parsed object against the Profile schema (either a local
validateProfile/validateProfileSchema function or an existing Zod schema in
core) and if validation fails call setValidationError with the validation
message and skip onSave; on success pass the validated object (not a blind cast)
to onSave. Ensure you reference the same key handler path where validateJson,
setValidationError, JSON.parse(...), and onSave(profileName, updatedProfile) are
used so the validation gate replaces the direct cast.
In @packages/cli/src/ui/hooks/useProfileManagement.ts:
- Around line 101-109: The code reads currentProfile from
runtime.getActiveProviderStatus() via a wrong type assertion, which is
unreliable; replace that logic to obtain the profile name from
runtime.getRuntimeDiagnosticsSnapshot()?.profileName and pass it to
setActiveProfileName (falling back to null if undefined), keeping the try/catch
behavior; update references in this block (getActiveProviderStatus,
setActiveProfileName) to use getRuntimeDiagnosticsSnapshot and ensure
activeProfileName state is set from its profileName field.
🧹 Nitpick comments (9)
packages/cli/src/ui/commands/profileCommand.test.ts (1)
158-162: LGTM! Test updated to reflect dialog-based interaction.The test correctly validates the architectural shift from text-based profile listing to an interactive dialog flow, now asserting that the command returns a dialog action with type
'profileList'.💡 Optional cleanup: Consider removing unused mock setup
The
listSavedProfilesmock is still configured on line 35 but is no longer verified in this test since the list command now opens a dialog instead of printing profile names. If this mock isn't used by other tests in this suite, consider removing it from the beforeEach hook for clarity.packages/cli/src/ui/hooks/useProfileManagement.ts (1)
190-196: Consider usingMessageType.WARNINGfor warning messages.Warnings are currently displayed using
MessageType.INFO, which may not visually distinguish them from informational messages. UsingMessageType.WARNINGwould provide better visual feedback to users.♻️ Optional improvement
for (const warning of result.warnings ?? []) { addMessage({ - type: MessageType.INFO, + type: MessageType.WARNING, content: `\u26A0 ${warning}`, timestamp: new Date(), }); }packages/cli/src/ui/components/ProfileDetailDialog.tsx (2)
28-37: Use the existing type guard from core instead of duplicating.The
isLoadBalancerProfiletype guard is already exported from@vybestack/llxprt-code-core. Importing it would reduce duplication and ensure consistency with the canonical type definition.♻️ Suggested fix
-import type { Profile } from '@vybestack/llxprt-code-core'; +import { isLoadBalancerProfile, type Profile } from '@vybestack/llxprt-code-core'; -// Type guard for load balancer profile -function isLoadBalancerProfile( - profile: Profile, -): profile is Profile & { - type: 'loadbalancer'; - profiles: string[]; - policy: string; -} { - return profile.type === 'loadbalancer'; -}
199-205: Sensitive key filtering may have false positives.The filter uses
key.includes('key')which would also match legitimate keys like'keyboard','apiKeyRetryCount', etc. Similarly,includes('auth')would match'author'. Consider using more specific patterns or an explicit denylist.♻️ Consider a more precise filter
{Object.entries(profile.ephemeralSettings) .filter( - ([key]) => - !key.includes('key') && - !key.includes('auth') && - !key.includes('secret'), + ([key]) => { + const lower = key.toLowerCase(); + // Explicit sensitive key patterns + const sensitivePatterns = [ + 'apikey', 'api-key', 'api_key', + 'authkey', 'auth-key', 'auth_key', + 'secret', 'password', 'token', + 'credential', 'keyfile', + ]; + return !sensitivePatterns.some(p => lower.includes(p)); + }, )packages/cli/src/ui/hooks/slashCommandProcessor.ts (1)
503-520: Silent failure whendialogDatavalidation fails.Both
profileDetailandprofileEditorcases return{ type: 'handled' }whendialogDatais missing or malformed, without logging an error or informing the user. This could make debugging issues difficult.Consider adding a warning log or error message when validation fails:
♻️ Optional improvement for profileDetail
case 'profileDetail': if ( result.dialogData && typeof result.dialogData === 'object' && 'profileName' in result.dialogData && typeof (result.dialogData as { profileName: unknown }) .profileName === 'string' ) { const profileName = ( result.dialogData as { profileName: string } ).profileName; slashCommandLogger.log( () => `opening profileDetail for ${profileName}`, ); // Pass true for openedDirectly since this came from /profile show actions.viewProfileDetail(profileName, true); + } else { + slashCommandLogger.warn( + () => 'profileDetail dialog missing valid profileName in dialogData', + ); } return { type: 'handled' };Also applies to: 521-538
packages/cli/src/ui/components/ProfileInlineEditor.tsx (2)
200-216: Duplicate error messages displayed for JSON validation failures.When
validateJsonis non-null, both the generic error block (lines 201-207) and the JSON-specific error block (lines 210-216) can show similar error messages. IfvalidationErroris set from a failed save attempt (line 115 or 122), andvalidateJsonalso has an error, the user sees two error messages.♻️ Consolidate error display
{/* Error display */} - {(validationError || externalError) && ( + {(validationError || externalError) && !validateJson && ( <Box marginBottom={1}> <Text color={SemanticColors.status.error}> Error: {validationError || externalError} </Text> </Box> )} {/* JSON validation status */} {validateJson && ( <Box marginBottom={1}> <Text color={SemanticColors.status.error}> JSON Error: {validateJson} </Text> </Box> )}
89-92: Backspace only removes from end of buffer - no mid-line cursor support.The edit buffer acts as an append-only input with backspace removing from the end. Users cannot position a cursor within the line to make edits in the middle. This may be intentional for simplicity, but could limit usability for editing complex JSON values.
Consider noting this limitation in the instructions or, if time permits, adding cursor position tracking within the edit buffer for mid-line edits.
packages/cli/src/ui/components/ProfileListDialog.tsx (1)
129-133: Vim 'l' key conflict with quick load action.In navigation mode, 'l' is mapped to load the selected profile (line 121-123), but vim users expect 'l' to mean "move right". The current behavior overrides vim semantics. Consider using a different key for quick load (e.g., 'L' uppercase or 'Enter' which already opens details).
This is a minor UX consideration since Enter already provides access to profile actions via the detail dialog.
packages/cli/src/ui/commands/profileCommand.ts (1)
969-971: Silent catch blocks may hide legitimate errors.Both
showCommandandeditCommandcatch errors from profile existence checks and continue to open the dialog anyway, with the comment "the dialog will show the error." However, iflistProfiles()throws due to a filesystem or permission error, the dialog may fail to load the profile and show a confusing error message that doesn't indicate the root cause.♻️ Consider logging or differentiating error types
try { const profiles = await listProfiles(); if (!profiles.includes(profileName)) { return { type: 'message', messageType: 'error', content: `Profile '${profileName}' not found. Use /profile list to see available profiles.`, }; } } catch (error) { - // Continue anyway, the dialog will show the error + // Log the error for debugging but continue to let the dialog handle it + logger.warn(() => `Failed to verify profile existence: ${error instanceof Error ? error.message : String(error)}`); }Also applies to: 1035-1037
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
packages/cli/src/runtime/runtimeSettings.tspackages/cli/src/ui/AppContainer.tsxpackages/cli/src/ui/commands/profileCommand.test.tspackages/cli/src/ui/commands/profileCommand.tspackages/cli/src/ui/commands/types.tspackages/cli/src/ui/components/DialogManager.tsxpackages/cli/src/ui/components/ProfileDetailDialog.tsxpackages/cli/src/ui/components/ProfileInlineEditor.tsxpackages/cli/src/ui/components/ProfileListDialog.tsxpackages/cli/src/ui/contexts/RuntimeContext.tsxpackages/cli/src/ui/contexts/UIActionsContext.tsxpackages/cli/src/ui/contexts/UIStateContext.tsxpackages/cli/src/ui/hooks/slashCommandProcessor.tspackages/cli/src/ui/hooks/useEditorSettings.test.tsxpackages/cli/src/ui/hooks/useProfileManagement.tspackages/cli/src/ui/layouts/DefaultAppLayout.tsxpackages/cli/src/ui/reducers/appReducer.test.tspackages/cli/src/ui/reducers/appReducer.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-09T07:46:06.289Z
Learnt from: sogoiii
Repo: vybestack/llxprt-code PR: 1052
File: packages/cli/src/ui/components/ProfileCreateWizard/utils.ts:35-40
Timestamp: 2026-01-09T07:46:06.289Z
Learning: In the Profile Create Wizard (packages/cli/src/ui/components/ProfileCreateWizard), provider values are always passed as `provider.value` from provider objects that originate from PROVIDER_OPTIONS, ensuring consistent provider ID lookups without needing normalization.
Applied to files:
packages/cli/src/ui/components/DialogManager.tsx
📚 Learning: 2025-12-12T23:30:35.260Z
Learnt from: acoliver
Repo: vybestack/llxprt-code PR: 779
File: packages/cli/src/ui/commands/__tests__/profileCommand.failover.test.ts:47-136
Timestamp: 2025-12-12T23:30:35.260Z
Learning: For load balancer profiles, the policy parameter is required and must be explicitly specified. The command structure is: `/profile save loadbalancer <name> <policy> <profile1> <profile2> ...` where policy must be either 'roundrobin' or 'failover'.
Applied to files:
packages/cli/src/ui/commands/profileCommand.ts
📚 Learning: 2025-12-12T15:10:34.317Z
Learnt from: acoliver
Repo: vybestack/llxprt-code PR: 0
File: :0-0
Timestamp: 2025-12-12T15:10:34.317Z
Learning: For load balancer profiles, configuration options should be stored in ephemeralSettings rather than as command-line flags. The command should be simple with sensible defaults, and advanced users can edit the profile's ephemeralSettings for customization.
Applied to files:
packages/cli/src/ui/commands/profileCommand.ts
🧬 Code graph analysis (6)
packages/cli/src/ui/components/ProfileListDialog.tsx (3)
packages/cli/src/ui/hooks/useKeypress.ts (1)
useKeypress(23-41)packages/cli/test-utils/ink-stub.ts (2)
Box(22-22)Text(23-23)packages/cli/src/ui/colors.ts (1)
SemanticColors(83-153)
packages/cli/src/ui/components/ProfileDetailDialog.tsx (3)
packages/core/src/types/modelParams.ts (2)
Profile(204-204)isLoadBalancerProfile(209-213)packages/cli/src/ui/hooks/useKeypress.ts (1)
useKeypress(23-41)packages/cli/src/ui/colors.ts (1)
SemanticColors(83-153)
packages/cli/src/ui/contexts/UIActionsContext.tsx (1)
scripts/test-acp-integration.mjs (1)
profileName(10-10)
packages/cli/src/ui/components/ProfileInlineEditor.tsx (3)
packages/core/src/types/modelParams.ts (1)
Profile(204-204)packages/cli/src/ui/hooks/useKeypress.ts (1)
useKeypress(23-41)packages/cli/src/ui/colors.ts (1)
SemanticColors(83-153)
packages/cli/src/ui/components/DialogManager.tsx (4)
packages/cli/src/ui/components/ProfileListDialog.tsx (1)
ProfileListDialog(33-356)packages/cli/src/ui/components/ProfileDetailDialog.tsx (1)
ProfileDetailDialog(39-312)packages/core/src/types/modelParams.ts (1)
Profile(204-204)packages/cli/src/ui/components/ProfileInlineEditor.tsx (1)
ProfileInlineEditor(23-298)
packages/cli/src/ui/commands/profileCommand.ts (3)
packages/cli/src/ui/commands/types.ts (4)
SlashCommand(210-240)CommandContext(32-98)MessageActionReturn(119-123)OpenDialogActionReturn(128-148)packages/cli/src/ui/commands/schema/types.ts (1)
CommandArgumentSchema(61-63)packages/core/src/config/profileManager.ts (1)
listProfiles(188-206)
🪛 GitHub Actions: LLxprt Code CI
packages/cli/src/ui/components/ProfileDetailDialog.tsx
[error] 1-1: Prettier formatting failed during 'npm run format'. The diff shows a type signature change in isLoadBalancerProfile. Run 'prettier --write' (as part of 'npm run format') to fix code style issues.
🔇 Additional comments (26)
packages/cli/src/ui/hooks/useEditorSettings.test.tsx (1)
66-68: LGTM! Mock state updated to match new AppState structure.The three new profile dialog flags are correctly added to the mock state, ensuring tests remain aligned with the expanded dialog surface.
packages/cli/src/ui/layouts/DefaultAppLayout.tsx (1)
166-168: LGTM! Dialog visibility checks properly extended.The three new profile dialogs are correctly integrated into the
dialogsVisibleflag, ensuring the main UI is appropriately hidden when any profile management dialog is open.packages/cli/src/ui/commands/types.ts (1)
143-145: LGTM! Dialog union type correctly expanded.The three new profile dialog identifiers are properly added to the
OpenDialogActionReturntype, enabling command actions to open profile management dialogs in a type-safe manner.packages/cli/src/ui/reducers/appReducer.ts (1)
25-27: LGTM! Reducer state consistently extended for profile dialogs.The three new profile dialog identifiers are correctly added across all necessary locations:
OPEN_DIALOGandCLOSE_DIALOGaction payloadsAppState.openDialogsinterfaceinitialAppState(properly initialized tofalse)The existing reducer logic automatically handles these new dialog keys through computed property access, requiring no additional case handling.
Also applies to: 42-44, 64-66, 92-94
packages/cli/src/runtime/runtimeSettings.ts (1)
1210-1213: LGTM!The new
getProfileByNamefunction is correctly implemented and follows the existing pattern used by other profile functions in this file (e.g.,loadProfileByName,saveProfileSnapshot). Each function creates a newProfileManagerinstance, which is consistent with the stateless approach used throughout.packages/cli/src/ui/contexts/RuntimeContext.tsx (1)
29-29: LGTM!The new
getProfileByNamefunction is properly imported and added to the runtime functions object, making it available through the runtime API facade. The placement alongside other profile-related functions (loadProfileByName,deleteProfileByName,listSavedProfiles) maintains good organization.Also applies to: 81-81
packages/cli/src/ui/reducers/appReducer.test.ts (1)
29-31: LGTM!The test file is correctly updated to include the new profile dialog flags (
profileList,profileDetail,profileEditor) in both the initial state assertion and the immutability test. The placement is consistent with the production reducer changes.packages/cli/src/ui/components/DialogManager.tsx (2)
273-286: LGTM!The
ProfileListDialogintegration correctly passes all required props with appropriate nullish coalescing for optional values.
288-305: LGTM!The
ProfileDetailDialogintegration properly handles the nullable state values and correctly computesisDefaultandisActivebased on profile name comparisons.packages/cli/src/ui/hooks/useProfileManagement.ts (1)
27-56: Consider memoizing derived state values.The hook reads
appState.openDialogs.*on every render. While this is fine, consider whether these could benefit from memoization if the component tree is large. This is a minor optimization consideration.packages/cli/src/ui/components/ProfileDetailDialog.tsx (2)
55-89: LGTM!The keyboard handling logic is well-structured with proper state management for the delete confirmation flow. The
useKeypresshook is correctly disabled during loading state.
29-36: Pipeline failure: Prettier formatting required.The CI pipeline indicates a Prettier formatting failure related to the type signature in
isLoadBalancerProfile. Runnpm run formatto fix code style issues.#!/bin/bash # Check the specific formatting issue cd packages/cli && npx prettier --check src/ui/components/ProfileDetailDialog.tsx⛔ Skipped due to learnings
Learnt from: CR Repo: vybestack/llxprt-code PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-11-25T16:56:18.980Z Learning: Before reporting a task as finished, run `npm run format` from the repository root and ensure it succeeds (exit code 0)Learnt from: acoliver Repo: vybestack/llxprt-code PR: 779 File: packages/cli/src/ui/commands/__tests__/profileCommand.failover.test.ts:47-136 Timestamp: 2025-12-12T23:30:35.260Z Learning: For load balancer profiles, the policy parameter is required and must be explicitly specified. The command structure is: `/profile save loadbalancer <name> <policy> <profile1> <profile2> ...` where policy must be either 'roundrobin' or 'failover'.packages/cli/src/ui/hooks/slashCommandProcessor.ts (2)
62-64: LGTM!The new action signatures in
SlashCommandProcessorActionscorrectly match the expected parameters for the profile management dialogs.
497-502: LGTM!The
profileListcase correctly logs and invokes the dialog action.packages/cli/src/ui/contexts/UIStateContext.tsx (1)
69-107: Well-structured state additions for profile management.The new dialog flags and profile management data block follow existing patterns in the UIState interface. The
profileListItemstype correctly captures the profile metadata needed for the list view, and usingunknownforselectedProfileDataappropriately accommodates theProfileunion type from the core package.packages/cli/src/ui/components/ProfileInlineEditor.tsx (1)
69-168: Keyboard navigation and edit mode logic is well-implemented.The dual-mode (editing vs navigation) keypress handling is clean. The vim-style navigation (j/k, g/G) complements arrow key navigation, and the scroll offset tracking ensures the cursor stays visible. The escape key properly exits edit mode without committing changes.
packages/cli/src/ui/contexts/UIActionsContext.tsx (1)
80-94: Comprehensive profile management actions added.The new actions provide a complete API for profile dialog orchestration. The
openedDirectlyparameter onviewProfileDetailandopenProfileEditorenables proper ESC navigation semantics (return to list vs close entirely) as documented in the PR objectives.packages/cli/src/ui/components/ProfileListDialog.tsx (3)
71-76: Navigation boundary handling is correct.The
movefunction properly clampsnextto valid indices (0 tofilteredProfiles.length - 1), preventing out-of-bounds access. The bounds check handles empty arrays correctly sincenext >= 0becomesnext >= -1which still clamps to 0.
186-196: Grid rendering uses appropriate keys.Row keys use the row index
r, and item keys inrenderItemuseprofile.namewhich should be unique within the profiles array. This ensures stable React reconciliation.
33-56: Well-structured responsive dialog with search and navigation modes.The dual-mode interaction (search vs navigation) with Tab to switch is intuitive. The filtered profiles with real-time search, visual indicators for active/default/load-balancer profiles, and responsive column layout provide a good user experience.
packages/cli/src/ui/commands/profileCommand.ts (3)
947-957: Profile name extraction handles quoted and unquoted names correctly.The regex
^"([^"]+)"$properly captures the content within quotes whiletrimmedArgsserves as the fallback for unquoted names. This is consistent with the patterns used inloadCommandanddeleteCommand.
898-913: List command updated to open interactive dialog.The change from returning a message with profile list to returning a
profileListdialog aligns with the PR objective of providing an interactive profile management UI. The logger call helps with debugging.
928-978: Show and edit commands properly integrate with dialog system.Both commands validate the profile name format, check for profile existence, and return appropriate dialog targets with
dialogDatacontaining the profile name. The return type unionMessageActionReturn | OpenDialogActionReturncorrectly reflects the possible outcomes.Also applies to: 994-1045
packages/cli/src/ui/AppContainer.tsx (3)
1203-1231: Profile management hook properly integrated.The
useProfileManagementhook is correctly initialized withaddMessageandappStateprops, consistent with other dialog hooks likeuseLoadProfileDialog. All returned values are appropriately destructured and used in the component.
2369-2378: Dependency array for uiActions includes all profile management callbacks.The useMemo dependency array properly includes all ten new profile management callbacks, ensuring the memoized object updates when any of these functions change.
2076-2108: UIState correctly exposes profile management data.The profile dialog flags and data are properly included in the UIState object, following the established patterns for other dialogs. This enables downstream components like DialogManager and DefaultAppLayout to access the profile management state.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @packages/cli/src/ui/components/ProfileDetailDialog.tsx:
- Around line 28-35: The current type guard isLoadBalancerProfile only checks
profile.type and then narrows to expect profile.profiles and profile.policy;
update isLoadBalancerProfile to perform runtime validation that profile.profiles
exists and is an array (e.g., Array.isArray(profile.profiles)) and that every
element is a string, and that profile.policy exists and is a string (typeof
profile.policy === 'string'), returning true only if all checks pass so later
uses of profile.policy and profile.profiles (e.g., where code accesses
profile.policy and profile.profiles) are safe.
- Around line 193-213: The current filtering in ProfileDetailDialog over
profile.ephemeralSettings is case-sensitive and can miss sensitive keys like
API_KEY or AuthToken; update the filter that currently does key.includes('key')/
'auth'/'secret' to normalize the key (e.g., const k = key.toLowerCase()) and
check k.includes('key'|'auth'|'secret') so matching is case-insensitive, keep
the subsequent undefined/null filter and slice(0,10) as-is, and ensure the same
normalized key is used only for filtering (not displayed) so displayed labels
remain unchanged.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/cli/src/ui/components/ProfileDetailDialog.tsx
⏰ Context from checks skipped due to timeout of 270000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test (windows-latest, 24.x)
- GitHub Check: Test (macos-latest, 24.x)
- GitHub Check: Test (ubuntu-latest, 24.x)
🔇 Additional comments (6)
packages/cli/src/ui/components/ProfileDetailDialog.tsx (6)
7-12: LGTM!The imports are clean and properly structured for React 19.2 and Ink 6.4.6.
14-26: LGTM!The props interface is well-designed with clear callback signatures and appropriate optional flags.
50-87: LGTM!The keyboard navigation implementation is well-structured with proper guard conditions and a safe delete confirmation flow. The
isActive: !isLoadingflag correctly prevents key handling during loading states.
89-121: LGTM!Loading and error states provide clear feedback with appropriate semantic colors and user guidance.
234-261: LGTM!The delete confirmation overlay provides clear visual feedback with appropriate width constraints and user guidance.
263-309: LGTM!The main render provides a clean layout with responsive width handling, clear status indicators, and comprehensive action hints.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/cli/src/ui/hooks/useProfileManagement.ts (2)
68-86: Consider improving fallback handling for corrupted profiles.When profile loading fails (line 78-84), the fallback defaults to
type: 'standard', which could be misleading if the profile is actually a load-balancer that failed to load due to corruption. Consider adding a distinct indicator (e.g.,type: 'unknown'or an error flag) so users can identify problematic profiles in the list.💡 Suggested approach
} catch { - // If we can't load the profile, just return basic info + // If we can't load the profile, return with error indicator return { name, - type: 'standard', + type: 'standard', // Could add an 'error' flag to ProfileListItem } as ProfileListItem; }This would require updating
ProfileListIteminterface to include an optionalloadError?: booleanfield.
309-331: Unsafe type assertion bypasses validation.Line 315 casts
updatedProfile as Profilewithout validation. WhileProfileManager.saveProfilemay perform internal validation, the type assertion here circumvents TypeScript's type safety. The PR objectives mention "profile schema validation before saving" — if that validation is withinProfileManager.saveProfile, consider letting any validation errors propagate more explicitly.Additionally, instantiating
new ProfileManager()on each save is fine but could be optimized by reusing an instance if performance becomes a concern.💡 Suggested improvement
const saveProfile = useCallback( async (profileName: string, updatedProfile: unknown) => { try { + // Type guard before save - ProfileManager.saveProfile will also validate + if ( + typeof updatedProfile !== 'object' || + updatedProfile === null || + !('version' in updatedProfile) || + !('type' in updatedProfile) + ) { + setProfileError('Invalid profile structure: missing version or type'); + return; + } // Use ProfileManager directly to save const manager = new ProfileManager(); await manager.saveProfile(profileName, updatedProfile as Profile);This adds a pre-check that aligns with the validation already present in
ProfileInlineEditorbeforeonSaveis called.packages/cli/src/ui/components/ProfileInlineEditor.tsx (2)
89-92:deletekey behaves identically tobackspace— may be unexpected.Both
backspaceanddeleteremove the last character. Typically,deleteremoves the character after the cursor position (forward delete), whilebackspaceremoves the character before it. Since this is a line-based editor without cursor position within the line, this simplification is acceptable, but users familiar with standard text editing may finddeletebehavior surprising.Consider documenting this behavior or, if you track cursor position within the edit buffer in the future, differentiating the two keys.
211-227: Consider consolidating error displays.Both the
validationError || externalErrorblock (lines 212-218) and thevalidateJsonblock (lines 221-227) may display simultaneously. IfvalidateJsonalready captures JSON parse errors, andvalidationErroris set from the same source during save, users might see duplicate or confusing error messages.💡 Suggested consolidation
{/* Error display */} - {(validationError || externalError) && ( + {(validationError || externalError || validateJson) && ( <Box marginBottom={1}> <Text color={SemanticColors.status.error}> - Error: {validationError || externalError} + Error: {validationError || externalError || validateJson} </Text> </Box> )} - - {/* JSON validation status */} - {validateJson && ( - <Box marginBottom={1}> - <Text color={SemanticColors.status.error}> - JSON Error: {validateJson} - </Text> - </Box> - )}This reduces visual clutter while maintaining all error sources.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/cli/src/ui/components/ProfileDetailDialog.tsxpackages/cli/src/ui/components/ProfileInlineEditor.tsxpackages/cli/src/ui/hooks/useProfileManagement.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/cli/src/ui/components/ProfileDetailDialog.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
packages/cli/src/ui/components/ProfileInlineEditor.tsx (4)
packages/core/src/types/modelParams.ts (1)
Profile(204-204)packages/cli/src/ui/hooks/useKeypress.ts (1)
useKeypress(23-41)packages/cli/test-utils/ink-stub.ts (2)
Box(22-22)Text(23-23)packages/cli/src/ui/colors.ts (1)
SemanticColors(83-153)
packages/cli/src/ui/hooks/useProfileManagement.ts (7)
packages/cli/src/ui/contexts/RuntimeContext.tsx (1)
useRuntimeApi(189-191)packages/cli/src/ui/components/ProfileListDialog.tsx (1)
ProfileListItem(14-21)packages/core/src/types/modelParams.ts (1)
Profile(204-204)packages/cli/src/runtime/runtimeSettings.ts (1)
setDefaultProfileName(1223-1226)packages/core/src/providers/LoggingProviderWrapper.ts (1)
name(313-315)scripts/test-acp-integration.mjs (1)
profileName(10-10)packages/core/src/config/profileManager.ts (2)
deleteProfile(212-223)ProfileManager(24-373)
🔇 Additional comments (9)
packages/cli/src/ui/hooks/useProfileManagement.ts (5)
1-16: LGTM!License header, imports, and debug logger setup are appropriate for the module.
27-56: LGTM!State management structure is well-organized. The
detailOpenedDirectlyandeditorOpenedDirectlyflags are a clean approach for preserving navigation context between dialogs.
118-174: LGTM!Dialog orchestration logic correctly handles both direct-access and list-originated navigation flows. The
detailOpenedDirectlyflag appropriately controls whether to return to the list dialog on close.
176-208: LGTM!Profile loading with info messages and warnings display is well-implemented. The dialog cleanup after loading is appropriate.
333-364: LGTM!The return object is well-structured, grouping state and actions by their associated dialogs for clear API ergonomics.
packages/cli/src/ui/components/ProfileInlineEditor.tsx (4)
1-21: LGTM!License, imports, and props interface are well-defined. The
Profiletype import ensures type safety for the component's data contract.
45-67: LGTM!Scroll viewport management and real-time JSON validation via
useMemoare well-implemented. The validation approach provides immediate feedback to users as they edit.
138-176: Navigation implementation is functional.The vim-style navigation (
j/kfor up/down,g/Gfor home/end) works correctly. Note that standard vim usesggfor start (not singleg), but for a simple line-based editor this simplification is reasonable.
229-308: LGTM!The editor rendering is well-structured with clear visual indicators for the current line, edit mode caret, scroll position, and contextual instructions. The responsive width handling and semantic color usage provide good UX.
| try { | ||
| const updatedProfile = JSON.parse(lines.join('\n')); | ||
| // Basic type validation - Profile must have version and type at minimum | ||
| if (typeof updatedProfile !== 'object' || updatedProfile === null) { | ||
| setValidationError('Invalid profile: must be an object'); | ||
| return; | ||
| } | ||
| if (!('version' in updatedProfile) || !('type' in updatedProfile)) { | ||
| setValidationError( | ||
| 'Invalid profile: missing required fields (version, type)', | ||
| ); | ||
| return; | ||
| } | ||
| onSave(profileName, updatedProfile as Profile); | ||
| } catch (e) { | ||
| setValidationError(e instanceof Error ? e.message : 'Invalid JSON'); | ||
| } | ||
| return; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find ProfileManager and its saveProfile method
fd -t f "ProfileManager" --exec cat {} \;Repository: vybestack/llxprt-code
Length of output: 47
🏁 Script executed:
# Search for saveProfile method definition
rg -n "saveProfile" --type ts --type tsx -A 10Repository: vybestack/llxprt-code
Length of output: 92
🏁 Script executed:
# Find the ProfileManager class/file
fd -t f -e ts -e tsx | xargs rg "class ProfileManager|export.*ProfileManager" -lRepository: vybestack/llxprt-code
Length of output: 107
🏁 Script executed:
cat packages/core/src/config/profileManager.tsRepository: vybestack/llxprt-code
Length of output: 11993
ProfileManager.saveProfile lacks validation—incomplete StandardProfiles can be saved silently.
The minimal client-side validation at lines 118-135 (checking only version and type) is insufficient because ProfileManager.saveProfile performs no validation at all. It simply writes the profile to disk without checking required fields.
This creates a critical asymmetry:
ProfileManager.saveLoadBalancerProfilevalidates comprehensivelyProfileManager.saveProfilevalidates nothing for StandardProfiles- Required fields like
providerandmodelare only validated when the profile is loaded, not when saved
Users can save incomplete StandardProfiles, and validation errors only surface later when attempting to load them. The try-catch block at line 134 cannot help since saveProfile throws no errors.
Add type-specific validation in the UI before calling onSave, or add validation to ProfileManager.saveProfile to match saveLoadBalancerProfile.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @packages/cli/src/ui/components/ProfileDetailDialog.tsx:
- Around line 58-92: The key handler registered via useKeypress is processing
quick-action keys even when the dialog is in an error or missing-profile state;
modify the handler in ProfileDetailDialog (the useKeypress callback) to
early-return for all non-Escape keys if error || !profile (while still allowing
Esc and the delete-confirm flow when appropriate), e.g., check error || !profile
near the top of the callback before handling 'l','e','d','s' and the
confirmDelete key sequences so onLoad/onEdit/onDelete/onSetDefault cannot fire
on error screens; ensure isActive behavior remains unchanged.
In @packages/cli/src/ui/components/ProfileInlineEditor.tsx:
- Around line 32-48: The editor state is only initialized from profile once, so
when profile or profileName changes while the component stays mounted the UI
shows stale JSON; add a useEffect that watches [profile, profileName] and calls
setLines(formatProfile(profile)) and resets cursorLine (0), isEditing (false),
editBuffer (''), validationError (null), hasChanges (false) and scrollOffset (0)
so the inline editor fully reinitializes when the active profile changes;
reference formatProfile, setLines, setCursorLine, setIsEditing, setEditBuffer,
setValidationError, setHasChanges, and setScrollOffset.
- Around line 69-179: The key handler in the useKeypress callback inside
ProfileInlineEditor.tsx currently checks key.insertable directly, which is
optional and can be undefined for paste/buffered events; update the conditional
in the printable-character branch (the if that tests key.sequence, typeof
key.sequence === 'string', !key.ctrl, !key.meta, key.insertable) to treat
undefined as insertable (e.g., replace key.insertable with a check like
key.insertable !== false) so paste and other non-standard input paths are
accepted while still preventing explicit non-insertable keys.
In @packages/cli/src/ui/components/ProfileListDialog.tsx:
- Around line 48-77: The selection index can become out-of-bounds when profiles
or filteredProfiles change; add a React.useEffect that watches profiles and
filteredProfiles.length (or profiles and searchTerm) and clamps setIndex to
Math.max(0, Math.min(index, filteredProfiles.length - 1)) (and set to 0 when
filteredProfiles.length === 0), and also harden move(delta) to handle
filteredProfiles.length === 0 by early-returning or setting next = 0; update
references to index usage to always guard against filteredProfiles[index] being
undefined.
In @packages/cli/src/ui/hooks/useProfileManagement.ts:
- Around line 58-117: loadProfiles currently sets profileError but the
ProfileListDialog has no error prop, so load failures show as an empty list;
update loadProfiles to surface failures to the UI by either (A) passing
profileError (or a boolean like hasProfileLoadError) into ProfileListDialog and
rendering an error state there (update the call site to send profileError and
add handling in ProfileListDialog), or (B) on the catch branch where
setProfileError(...) is called also call addMessage({ type: 'ERROR', title:
'Failed to load profiles', detail: error instanceof Error ? error.message :
undefined }) so the global message center shows the failure; reference
loadProfiles, profileError, setProfileError, ProfileListDialog, and addMessage
when making the change so the dialog or the global messaging is updated
consistently.
🧹 Nitpick comments (3)
packages/cli/src/ui/hooks/useProfileManagement.ts (1)
135-159: Potential stale async update when switching profiles quickly.
viewProfileDetail()can be invoked multiple times; the slowergetProfileByName()response can overwriteselectedProfilefor a newer selection. Consider guarding with a request id (useRef counter) or checkingprofileName === selectedProfileNamebeforesetSelectedProfile(profile).packages/cli/src/ui/components/ProfileDetailDialog.tsx (1)
198-223: Prefer an allowlist for “safe to display” settings (denylist can leak secrets).The
ephemeralSettingsfiltering is a substring-based denylist; it’s easy to miss sensitive keys (e.g., “credential”, “cookie”, “session”, provider-specific names), and it also doesn’t account for nested objects containing secrets.Suggestion: switch to an allowlist of known safe keys (or only show counts / keys, not values).
packages/cli/src/ui/components/ProfileListDialog.tsx (1)
144-151:loadErroris never shown, so corrupted profiles look normal.
ProfileListItemhasloadError?: booleananduseProfileManagement.loadProfiles()sets it, but the list only renders*,D,LB. Consider adding an indicator (e.g.,!) and/or styling (warning/error color), and blocking “load/detail” whenloadErroris true.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/cli/src/ui/components/ProfileDetailDialog.tsxpackages/cli/src/ui/components/ProfileInlineEditor.tsxpackages/cli/src/ui/components/ProfileListDialog.tsxpackages/cli/src/ui/hooks/useProfileManagement.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/cli/src/ui/components/ProfileListDialog.tsx (2)
packages/cli/src/ui/hooks/useKeypress.ts (1)
useKeypress(23-41)packages/cli/src/ui/colors.ts (1)
SemanticColors(83-153)
packages/cli/src/ui/components/ProfileInlineEditor.tsx (3)
packages/core/src/types/modelParams.ts (1)
Profile(204-204)packages/cli/src/ui/hooks/useKeypress.ts (1)
useKeypress(23-41)packages/cli/src/ui/colors.ts (1)
SemanticColors(83-153)
⏰ Context from checks skipped due to timeout of 270000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test (windows-latest, 24.x)
- GitHub Check: Test (macos-latest, 24.x)
- GitHub Check: Test (ubuntu-latest, 24.x)
🔇 Additional comments (1)
packages/cli/src/ui/hooks/useProfileManagement.ts (1)
59-117: No action needed. BothsaveProfile()and runtime profile operations (listSavedProfiles(),getProfileByName()) use the same defaultProfileManagerinitialization, which resolves to~/.llxprt/profiles. The storage backend and validation rules are unified.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @packages/cli/src/ui/components/ProfileDetailDialog.tsx:
- Around line 203-227: The current ProfileDetailDialog rendering of
profile.ephemeralSettings uses substring filtering to exclude secrets, which can
miss many sensitive keys; update the rendering logic in ProfileDetailDialog to
stop relying on substring exclusion and either 1) render only an explicit
allowlist of safe keys (e.g., a constant SAFE_EPHEMERAL_KEYS) or 2) mask all
values by default (e.g., show keys but replace value with "*****" or
"<redacted>") and only reveal a value via an explicit user action; ensure you
update the mapping code that iterates profile.ephemeralSettings (the
.filter/.map pipeline) to apply the allowlist or masking, keep the display limit
(.slice(0,10)), and avoid printing raw JSON.stringify(value) for any
non-allowlisted key.
In @packages/cli/src/ui/components/ProfileInlineEditor.tsx:
- Around line 69-78: The validationError state is not cleared when JSON becomes
valid or when lines change, causing stale error UI; update the component to
clear validationError whenever the input lines are edited or when validateJson
becomes null — e.g., inside the effect or handler that watches lines or
validateJson (the useMemo named validateJson and the state setter
setValidationError), call setValidationError(null) on any change/commit of lines
or immediately when validateJson flips to null (also apply the same fix in the
other similar blocks referenced around lines 123-147 and 222-229).
In @packages/cli/src/ui/components/ProfileListDialog.tsx:
- Around line 91-149: The search-input branch in the useKeypress handler
currently checks key.insertable truthily which drops input when insertable is
undefined; update the condition in ProfileListDialog's key handler (inside the
useKeypress callback that calls setSearchTerm) to use key.insertable !== false
instead of checking key.insertable, and apply the same change in the analogous
handlers in ProviderDialog and ProviderModelDialog so paste and multi-character
sequences are accepted consistently.
🧹 Nitpick comments (4)
packages/cli/src/ui/hooks/useProfileManagement.ts (2)
58-123: Consider propagating “load failed” vs “no profiles” to the list UI.
loadProfiles()setsprofileErroron failure, butProfileListDialog(as typed) can only show loading/empty states—so a load failure can look like “No saved profiles found”. Consider plumbing anerrorprop into the list dialog (or a dedicated “failed to load” state) to avoid misleading UX.
315-348: Use the runtime API for consistency with other profile-saving UI code.This hook uses
new ProfileManager()directly, but other UI code (useWelcomeOnboarding, profileCommand) saves profiles throughruntime.saveProfileSnapshot(). For architectural consistency, consider using the runtime API instead of direct ProfileManager instantiation.Note: The concern about different stores or encryption is not applicable here—all ProfileManager instances use the same default directory (
~/.llxprt/profiles) for both reads and writes.packages/cli/src/ui/components/ProfileDetailDialog.tsx (1)
28-40: Prefer the coreisLoadBalancerProfile()guard over a local re-implementation.There’s already an exported guard in
packages/core/src/types/modelParams.ts(isLoadBalancerProfile(profile): profile is LoadBalancerProfile). Reusing it reduces drift risk if the LB schema evolves.packages/cli/src/ui/components/ProfileListDialog.tsx (1)
151-197: Consider surfacingloadErroritems in the list UI.
ProfileListItem.loadErroris set by the hook, but the list currently doesn’t display it. Even a small marker (e.g.,[!]) or coloring would help users understand why detail/actions may fail for that entry.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/cli/src/ui/components/ProfileDetailDialog.tsxpackages/cli/src/ui/components/ProfileInlineEditor.tsxpackages/cli/src/ui/components/ProfileListDialog.tsxpackages/cli/src/ui/hooks/useProfileManagement.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/cli/src/ui/components/ProfileDetailDialog.tsx (3)
packages/core/src/types/modelParams.ts (2)
Profile(204-204)isLoadBalancerProfile(209-213)packages/cli/src/ui/hooks/useKeypress.ts (1)
useKeypress(23-41)packages/cli/src/ui/colors.ts (1)
SemanticColors(83-153)
packages/cli/src/ui/components/ProfileInlineEditor.tsx (3)
packages/core/src/types/modelParams.ts (1)
Profile(204-204)packages/cli/src/ui/hooks/useKeypress.ts (1)
useKeypress(23-41)packages/cli/src/ui/colors.ts (1)
SemanticColors(83-153)
⏰ Context from checks skipped due to timeout of 270000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test (windows-latest, 24.x)
- GitHub Check: Test (macos-latest, 24.x)
- GitHub Check: Test (ubuntu-latest, 24.x)
🔇 Additional comments (3)
packages/cli/src/ui/hooks/useProfileManagement.ts (1)
166-180: Back-navigation logic looks consistent; verify dialog sequencing doesn’t flicker.The “opened directly” flags +
closeDetailDialog()/closeEditor()flows look coherent (direct open exits; list/detail flow returns). Only thing to verify is whether DialogManager renders transient states between CLOSE/OPEN dispatches (visual flicker) on slower terminals.Also applies to: 292-313
packages/cli/src/ui/components/ProfileDetailDialog.tsx (1)
58-97: Key handling is well-scoped (Esc always works; actions blocked on error/missing profile).packages/cli/src/ui/components/ProfileInlineEditor.tsx (1)
80-190: Keymap + save guardrails look solid (includinginsertable !== false).
| {/* Ephemeral Settings - show non-sensitive ones */} | ||
| {profile.ephemeralSettings && ( | ||
| <Box flexDirection="column" marginBottom={1}> | ||
| <Text color={SemanticColors.text.secondary}>Settings:</Text> | ||
| {Object.entries(profile.ephemeralSettings) | ||
| .filter(([key]) => { | ||
| const k = key.toLowerCase(); | ||
| return ( | ||
| !k.includes('key') && | ||
| !k.includes('auth') && | ||
| !k.includes('secret') && | ||
| !k.includes('token') && | ||
| !k.includes('password') | ||
| ); | ||
| }) | ||
| .filter(([, value]) => value !== undefined && value !== null) | ||
| .slice(0, 10) // Limit displayed settings | ||
| .map(([key, value]) => ( | ||
| <Text key={key} color={SemanticColors.text.primary}> | ||
| {' '} | ||
| {key}: {JSON.stringify(value)} | ||
| </Text> | ||
| ))} | ||
| </Box> | ||
| )} |
There was a problem hiding this comment.
Potential secret leakage via ephemeralSettings key-substring filtering.
Filtering by substrings (key/auth/secret/token/password) will miss many common secret names (e.g., credential, session, jwt, bearer, cookie, private, etc.). Consider either (a) not rendering values at all, (b) masking values by default, or (c) using an allowlist of known-safe keys.
🤖 Prompt for AI Agents
In @packages/cli/src/ui/components/ProfileDetailDialog.tsx around lines 203 -
227, The current ProfileDetailDialog rendering of profile.ephemeralSettings uses
substring filtering to exclude secrets, which can miss many sensitive keys;
update the rendering logic in ProfileDetailDialog to stop relying on substring
exclusion and either 1) render only an explicit allowlist of safe keys (e.g., a
constant SAFE_EPHEMERAL_KEYS) or 2) mask all values by default (e.g., show keys
but replace value with "*****" or "<redacted>") and only reveal a value via an
explicit user action; ensure you update the mapping code that iterates
profile.ephemeralSettings (the .filter/.map pipeline) to apply the allowlist or
masking, keep the display limit (.slice(0,10)), and avoid printing raw
JSON.stringify(value) for any non-allowlisted key.
| // Validate JSON on changes | ||
| const validateJson = useMemo(() => { | ||
| try { | ||
| const jsonString = lines.join('\n'); | ||
| JSON.parse(jsonString); | ||
| return null; | ||
| } catch (e) { | ||
| return e instanceof Error ? e.message : 'Invalid JSON'; | ||
| } | ||
| }, [lines]); |
There was a problem hiding this comment.
Clear stale validationError once JSON is valid (or on any edit).
After a failed save, validationError can remain even when validateJson becomes null, because it’s not cleared on line commits / successful validation. Clearing it on lines change (or when validateJson flips to null) avoids sticky error UI.
Proposed change
const validateJson = useMemo(() => {
try {
const jsonString = lines.join('\n');
JSON.parse(jsonString);
return null;
} catch (e) {
return e instanceof Error ? e.message : 'Invalid JSON';
}
}, [lines]);
+ useEffect(() => {
+ if (!validateJson) {
+ setValidationError(null);
+ }
+ }, [validateJson]);Also applies to: 123-147, 222-229
🤖 Prompt for AI Agents
In @packages/cli/src/ui/components/ProfileInlineEditor.tsx around lines 69 - 78,
The validationError state is not cleared when JSON becomes valid or when lines
change, causing stale error UI; update the component to clear validationError
whenever the input lines are edited or when validateJson becomes null — e.g.,
inside the effect or handler that watches lines or validateJson (the useMemo
named validateJson and the state setter setValidationError), call
setValidationError(null) on any change/commit of lines or immediately when
validateJson flips to null (also apply the same fix in the other similar blocks
referenced around lines 123-147 and 222-229).
| useKeypress( | ||
| (key) => { | ||
| if (key.name === 'escape') { | ||
| if (isSearching && searchTerm.length > 0) { | ||
| setSearchTerm(''); | ||
| } else { | ||
| return onClose(); | ||
| } | ||
| } | ||
|
|
||
| if (isSearching || isNarrow) { | ||
| if (key.name === 'return') { | ||
| if (filteredProfiles.length > 0) { | ||
| if (isNarrow) { | ||
| return onViewDetail(filteredProfiles[index].name); | ||
| } | ||
| setIsSearching(false); | ||
| } | ||
| } else if (key.name === 'tab' && !isNarrow) { | ||
| setIsSearching(false); | ||
| } else if (key.name === 'backspace' || key.name === 'delete') { | ||
| setSearchTerm((prev) => prev.slice(0, -1)); | ||
| } else if ( | ||
| key.sequence && | ||
| typeof key.sequence === 'string' && | ||
| !key.ctrl && | ||
| !key.meta && | ||
| key.insertable | ||
| ) { | ||
| setSearchTerm((prev) => prev + key.sequence); | ||
| } | ||
| } else { | ||
| // Navigation mode | ||
| if (key.name === 'return' && filteredProfiles.length > 0) { | ||
| return onViewDetail(filteredProfiles[index].name); | ||
| } | ||
| if (key.name === 'tab') { | ||
| setIsSearching(true); | ||
| } | ||
| if (filteredProfiles.length === 0) { | ||
| return; | ||
| } | ||
| // Quick actions | ||
| if (key.sequence === 'l' && filteredProfiles.length > 0) { | ||
| return onSelect(filteredProfiles[index].name); | ||
| } | ||
| // Navigation | ||
| if (key.name === 'left') move(-1); | ||
| if (key.name === 'right') move(1); | ||
| if (key.name === 'up') move(-columns); | ||
| if (key.name === 'down') move(columns); | ||
| // Vim-style navigation | ||
| if (key.sequence === 'j') move(columns); | ||
| if (key.sequence === 'k') move(-columns); | ||
| if (key.sequence === 'h') move(-1); | ||
| } | ||
| }, | ||
| { isActive: !isLoading }, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "ProfileListDialog.tsx" | head -5Repository: vybestack/llxprt-code
Length of output: 119
🏁 Script executed:
git ls-files "**/*.tsx" "**/*.ts" | grep -i profileRepository: vybestack/llxprt-code
Length of output: 2664
🏁 Script executed:
cat -n packages/cli/src/ui/components/ProfileListDialog.tsx | sed -n '85,155p'Repository: vybestack/llxprt-code
Length of output: 2836
🏁 Script executed:
# Search for other uses of key.insertable to find the "editor" reference
rg "key\.insertable" --type ts --type tsx -B 2 -A 2Repository: vybestack/llxprt-code
Length of output: 92
🏁 Script executed:
rg "key\.insertable" -B 2 -A 2Repository: vybestack/llxprt-code
Length of output: 2203
🏁 Script executed:
# Search for editor-related keypress handling to find the reference
rg "insertable\s*!==" -B 3 -A 3Repository: vybestack/llxprt-code
Length of output: 1282
🏁 Script executed:
# Look for the useKeypress hook definition or any other components using insertable check
rg "insertable" --type tsRepository: vybestack/llxprt-code
Length of output: 1331
🏁 Script executed:
cat -n packages/cli/src/ui/contexts/KeypressContext.tsx | head -100Repository: vybestack/llxprt-code
Length of output: 3330
🏁 Script executed:
# Check what values insertable is actually set to in KeypressContext
rg "insertable\s*:" packages/cli/src/ui/contexts/KeypressContext.tsx -B 2 -A 1Repository: vybestack/llxprt-code
Length of output: 681
🏁 Script executed:
# Check the Key type definition to see if insertable can be undefined
rg "type.*Key\s*=|interface.*Key\s*{" packages/cli/src/ui/contexts/KeypressContext.tsx -A 15Repository: vybestack/llxprt-code
Length of output: 192
🏁 Script executed:
# Search for all places where broadcast is called to verify insertable is always set
rg "broadcast\(" packages/cli/src/ui/contexts/KeypressContext.tsx -B 3 -A 3 | head -80Repository: vybestack/llxprt-code
Length of output: 2038
🏁 Script executed:
# Check the complete condition for shouldInsert to understand when insertable could be undefined
rg "shouldInsert\s*=" packages/cli/src/ui/contexts/KeypressContext.tsx -B 5 -A 2Repository: vybestack/llxprt-code
Length of output: 324
🏁 Script executed:
# Search for the Key type definition more carefully
rg "insertable" packages/cli/src/ui/contexts/KeypressContext.tsx | grep -E "insertable\s*\?" | head -5Repository: vybestack/llxprt-code
Length of output: 88
🏁 Script executed:
# Look at ProfileInlineEditor to understand the context where insertable !== false is used
cat -n packages/cli/src/ui/components/ProfileInlineEditor.tsx | sed -n '1,50p'Repository: vybestack/llxprt-code
Length of output: 2016
🏁 Script executed:
# Search for all text input handling to see the pattern
rg "key.insertable" -B 5 -A 5Repository: vybestack/llxprt-code
Length of output: 4606
🏁 Script executed:
# Check if there are any cases where insertable might be undefined - look for key spread without explicit insertable
rg "broadcast.*\.\.\." packages/cli/src/ui/contexts/KeypressContext.tsx -A 1 | grep -v insertableRepository: vybestack/llxprt-code
Length of output: 85
🏁 Script executed:
# Verify the exact type definition of Key to understand if insertable is truly optional
cat -n packages/cli/src/ui/contexts/KeypressContext.tsx | grep -A 20 "export.*type.*Key"Repository: vybestack/llxprt-code
Length of output: 734
🏁 Script executed:
# Search for the Key interface/type definition
rg "^type Key|^interface Key" packages/cli/src/ui/contexts/KeypressContext.tsx -A 20Repository: vybestack/llxprt-code
Length of output: 541
🏁 Script executed:
# Also check the imports to see if Key is imported from readline
head -50 packages/cli/src/ui/contexts/KeypressContext.tsx | grep -E "import|Key"Repository: vybestack/llxprt-code
Length of output: 463
🏁 Script executed:
# Search for where Key is imported or defined in relation to readline
rg "Key\s*=" packages/cli/src/ui/contexts/KeypressContext.tsx -B 2 -A 2Repository: vybestack/llxprt-code
Length of output: 447
🏁 Script executed:
# Check if Key is extended or augmented anywhere in the file
cat -n packages/cli/src/ui/contexts/KeypressContext.tsx | sed -n '160,200p'Repository: vybestack/llxprt-code
Length of output: 1118
🏁 Script executed:
# Find what type of 'key' object is actually being passed through broadcast
rg "on\('keypress'" packages/cli/src/ui/contexts/KeypressContext.tsx -A 10 | head -30Repository: vybestack/llxprt-code
Length of output: 616
Change search input check to key.insertable !== false for consistency and paste support.
ProfileListDialog, ProviderDialog, and ProviderModelDialog all use key.insertable (truthy check) for search input, but ProfileInlineEditor and text-buffer.ts use the safer key.insertable !== false. Since insertable is optional on the Key interface, using the truthy check risks dropping input when insertable is undefined rather than explicitly false. This can break paste and multi-character sequences. Align these three components with the established pattern.
🤖 Prompt for AI Agents
In @packages/cli/src/ui/components/ProfileListDialog.tsx around lines 91 - 149,
The search-input branch in the useKeypress handler currently checks
key.insertable truthily which drops input when insertable is undefined; update
the condition in ProfileListDialog's key handler (inside the useKeypress
callback that calls setSearchTerm) to use key.insertable !== false instead of
checking key.insertable, and apply the same change in the analogous handlers in
ProviderDialog and ProviderModelDialog so paste and multi-character sequences
are accepted consistently.
…or, and direct command access - Add ProfileListDialog with search, grid layout, and keyboard navigation - Add ProfileDetailDialog showing profile info and management actions - Add ProfileInlineEditor for JSON profile editing with validation - Add useProfileManagement hook for dialog state and profile operations - Add /profile show <name> command for direct profile viewing - Add /profile edit <name> command for direct profile editing - Integrate profile dialogs into DialogManager and DefaultAppLayout - Update app reducer with profile dialog state management - Add proper dialog exit behavior (ESC closes to terminal or previous view)
- Strengthen type guard validation in isLoadBalancerProfile - Fix case-insensitive filtering for sensitive data (API_KEY, tokens, passwords) - Add profile schema validation before saving edited profiles - Use getRuntimeDiagnosticsSnapshot() for active profile retrieval - Preserve navigation context when saving edited profiles
- Add loadError flag to ProfileListItem for corrupted profile handling - Add profile schema validation in saveProfile before ProfileManager call - Consolidate error displays in ProfileInlineEditor to reduce visual clutter
- Prevent quick-action keys firing on error/missing profile screens - Reset editor state when profile changes without unmount - Fix key.insertable check to allow paste events - Clamp selection index when filtered list changes - Surface profile load failures via global message center
- Replace denylist with allowlist for ephemeralSettings display to prevent secret leakage - Clear stale validation errors when JSON becomes valid in ProfileInlineEditor - Use key.insertable !== false for consistent input handling and paste support - Add ProfileDialogData type definitions for type-safe dialog data
sogoiii
force-pushed
the
feat/profile-management-ui
branch
from
ef47bc4 to
fa10e3f
Compare
2 participants
Purpose
Implement the complete profile management UI as specified in PROF-001, providing users with an interactive, searchable profile interface that matches the existing
/providerand/modeldialog patterns. This enhancement includes searchable profile list, detailed view with quick actions, and direct command access via/profile showand/profile edit.Summary
This PR completes the Profile Management feature specification with the following components:
Changes
Components Added
ProfileListDialog.tsx
*(active),D(default),LB(load balancer)ProfileDetailDialog.tsx
ProfileInlineEditor.tsx
useProfileManagement.ts Hook
detailOpenedDirectlyandeditorOpenedDirectlyflagsCommands Added
/profile show <name>/profile edit <name>Integration
dialogsVisiblecheckNavigation & State Flow
List Dialog
Direct Show Command
Direct Edit Command
Keyboard Controls
Testing Checklist
/profile show <name>command works/profile edit <name>command worksImages
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.