fix: handle cli configuration errors - #4597
Conversation
🦋 Changeset detectedLatest commit: a6b6d6d 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 |
Add translations for cliMisconfigured and configureCli keys to all 21 supported locales.
There was a problem hiding this comment.
✅ No Issues Found
10 files reviewed | Confidence: 95% | Recommendation: Merge
Review Details
Files Reviewed:
src/core/kilocode/agent-manager/CliProcessHandler.ts- Core logic for detecting CLI configuration errorssrc/core/kilocode/agent-manager/AgentManagerProvider.ts- UI handling for configuration errorssrc/core/kilocode/agent-manager/CliOutputParser.ts- Parser updates for instructions fieldsrc/core/kilocode/agent-manager/CliSessionLauncher.ts- NewgetPrewarmedCliPath()methodsrc/core/kilocode/agent-manager/telemetry.ts- New telemetry typesrc/i18n/locales/en/kilocode.json- Localization strings- Test files (3 files with comprehensive coverage)
Checked: Security, bugs, performance, error handling, memory management
Highlights:
- ✅ Memory-safe buffer management with 64KB cap prevents unbounded growth
- ✅ Multiple fallback mechanisms for error detection (JSON parsing → raw output patterns)
- ✅ Proper type safety with
cli_configuration_erroradded to all relevant type definitions - ✅ Comprehensive test coverage for all new scenarios including edge cases
- ✅ Follows established codebase patterns and conventions
There was a problem hiding this comment.
✅ Previous Issues Addressed
The review comment from RSO regarding code complexity in handleProcessExit has been resolved in commit 122f44e. The code now correctly extracts configurationError to a local variable before clearing pendingProcess, making the logic cleaner and easier to follow.
No new issues found in the latest changes.
Review Details
Files Reviewed: src/core/kilocode/agent-manager/CliProcessHandler.ts (latest commit only)
Verified:
- Configuration error extraction follows the cleaner pattern suggested by RSO
- Memory-safe buffer management preserved
- Test coverage remains comprehensive
There was a problem hiding this comment.
✅ No New Issues
Changes since last review (commits b6b4ba10, fdb7b1dc) look good.
| Commit | Description | Assessment |
|---|---|---|
b6b4ba10 |
Simplify logic further | Minor refactoring, cleaner code |
fdb7b1dc |
Fix type errors | Type fixes in AgentManagerProvider.ts and CliSessionLauncher.ts |
Review Details (2 commits since last review)
Files Changed:
src/core/kilocode/agent-manager/CliProcessHandler.ts- Logic simplificationsrc/core/kilocode/agent-manager/AgentManagerProvider.ts- Type fixessrc/core/kilocode/agent-manager/CliSessionLauncher.ts- Type fixes
Verified:
- Configuration error detection logic remains correct
- Memory-safe buffer management preserved
- Test coverage comprehensive
- RSO's previous feedback addressed
Resolve conflict in CliOutputParser.ts: - Keep session_title_generated event from main - Keep instructions in welcome event comment from HEAD
Fix Agent Manager not showing error when CLI is misconfigured. When the CLI exits with a configuration error (e.g., missing kilocodeToken), the extension now detects this and shows an error popup with options to run
kilocode authorkilocode config.