feat(cmd): add --model flag to init command (#188)#190
Open
feloy wants to merge 2 commits intokortex-hub:mainfrom
Open
feat(cmd): add --model flag to init command (#188)#190feloy wants to merge 2 commits intokortex-hub:mainfrom
feloy wants to merge 2 commits intokortex-hub:mainfrom
Conversation
Add optional --model/-m flag to specify a model ID for the agent during workspace initialization. The flag takes precedence over any model defined in the agent's default settings files. - Add SetModel method to Agent interface - Implement SetModel for Claude (sets model in .claude.json) - Implement SetModel for Goose (sets GOOSE_MODEL in config.yaml) - Implement SetModel for Cursor (set model in .cursor/cli-config.json) - Call SetModel in manager.Add when model is specified - Update README and skills documentation Made-with: Cursor Co-Authored-By: Claude Code (Claude Sonnet 4.5) <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughAdds a model-override path to instance creation: a new Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant CLI as "kortex-cli init"
participant Manager as "instances.Manager"
participant Registry as "Agent Registry"
participant Agent as "Agent Implementation"
participant Settings as "Settings File"
User->>CLI: init --model claude-sonnet
CLI->>Manager: Add(AddOptions{Model: "claude-sonnet"})
Manager->>Registry: Get(agentName)
Registry-->>Manager: agentImpl
Manager->>Agent: SkipOnboarding(settings)
Agent->>Settings: read settings
Settings-->>Agent: config data
Agent-->>Manager: updated settings
Manager->>Agent: SetModel(settings, "claude-sonnet")
Agent->>Settings: unmarshal existing config
Settings-->>Agent: config data
Agent->>Agent: set model field to "claude-sonnet"
Agent->>Settings: marshal & write config
Agent-->>Manager: settings w/ model
Manager-->>CLI: instance registered
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/agent/claude.go (1)
64-70:⚠️ Potential issue | 🟠 MajorInitialize
configafter unmarshalling Claude JSON to handle null values.When a settings file contains the valid JSON literal
null, unmarshalling into a nil-initializedmap[string]interface{}leaves the map nil. Lines 70 and 131 then panic on map assignment instead of returning an error.🛠️ Suggested fix
var config map[string]interface{} if err := json.Unmarshal(existingContent, &config); err != nil { return nil, fmt.Errorf("failed to parse existing %s: %w", ClaudeJSONPath, err) } + if config == nil { + config = make(map[string]interface{}) + } // Set hasCompletedOnboarding config["hasCompletedOnboarding"] = truevar config map[string]interface{} if err := json.Unmarshal(existingContent, &config); err != nil { return nil, fmt.Errorf("failed to parse existing %s: %w", ClaudeSettingsPath, err) } + if config == nil { + config = make(map[string]interface{}) + } config["model"] = modelIDAlso applies to: Line 131
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/agent/claude.go` around lines 64 - 70, After json.Unmarshal into the local variable config in pkg/agent/claude.go, guard against the JSON literal null by initializing the map when it's nil (e.g., if config == nil then config = make(map[string]interface{})); apply the same nil-check/initialization before any map assignment sites (the subsequent assignment to config["hasCompletedOnboarding"] and the other assignment around the later unmarshal at the second location) so assignments won't panic and existing behavior continues.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/agent/cursor.go`:
- Around line 86-99: When json.Unmarshal into the local variable config may
produce a nil map (e.g., when the file contains the JSON literal null), guard
against a nil config before assigning keys: after the Unmarshal in
pkg/agent/cursor.go, check if config == nil and if so set config =
make(map[string]interface{}) so subsequent assignments like config["model"] =
... and config["hasChangedDefaultModel"] = true (which reference modelID and
CursorCLIConfigPath in the error message) won’t panic; update the block around
the Unmarshal and the config population to ensure config is always an
initialized map before use.
---
Outside diff comments:
In `@pkg/agent/claude.go`:
- Around line 64-70: After json.Unmarshal into the local variable config in
pkg/agent/claude.go, guard against the JSON literal null by initializing the map
when it's nil (e.g., if config == nil then config =
make(map[string]interface{})); apply the same nil-check/initialization before
any map assignment sites (the subsequent assignment to
config["hasCompletedOnboarding"] and the other assignment around the later
unmarshal at the second location) so assignments won't panic and existing
behavior continues.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 35de5905-924b-4f51-966d-934dd726199f
📒 Files selected for processing (15)
README.mdpkg/agent/agent.gopkg/agent/claude.gopkg/agent/claude_test.gopkg/agent/cursor.gopkg/agent/cursor_test.gopkg/agent/goose.gopkg/agent/goose_test.gopkg/agent/registry_test.gopkg/cmd/init.gopkg/cmd/init_test.gopkg/instances/manager.gopkg/instances/manager_test.goskills/working-with-config-system/SKILL.mdskills/working-with-instances-manager/SKILL.md
Signed-off-by: Philippe Martin <phmartin@redhat.com> Co-Authored-By: Claude Code (Claude Sonnet 4.5) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add optional --model/-m flag to specify a model ID for the agent during workspace initialization. The flag takes precedence over any model defined in the agent's default settings files.
Closes #188