fix/openclaw config merge#53
Conversation
When installing Kimchi via 'curl ... | bash', stdin is a pipe from curl and is already at EOF. This caused the first-run API key prompt to immediately fail with 'error: read API key: EOF'. Redirect kimchi's stdin from /dev/tty when the script detects it is running in a non-TTY context, so the interactive prompt still works. If no TTY is available, skip auto-launch and print a helpful message. Co-Authored-By: Kimchi <noreply@kimchi.dev>
OpenClaw's config set defaults to replace mode, which causes
the setup to fail when users already have model entries in
agents.defaults.models or agents.defaults.model.fallbacks.
Changes across all three code paths:
1. Batch JSON mode (writeOpenClawViaCLI):
- Split batch to only cover non-conflicting fields
(provider block + primary).
- Use separate openclaw config set --merge calls for
agents.defaults.model.fallbacks and agents.defaults.models.
2. Sequential CLI mode (writeOpenClawViaCLISequential):
- Add --merge flag for agent.defaults.model.fallbacks.
- Add --merge flag for agent.defaults.models.
3. Direct file-writing mode (writeOpenClawDirect):
- Merge Kimchi model catalog into existing defaults["models"]
instead of replacing it entirely.
- Merge into existing defaults["model"] map instead of
overwriting, preserving keys like temperature and max_tokens.
- Append Kimchi fallbacks to existing fallbacks array.
Co-Authored-By: Kimchi <noreply@kimchi.dev>
Kimchi Code Review
Summary📊 Review Score: 85/100 (overall code quality — 0 lowest, 100 highest) 🧪 Tests: no — No test files were modified or added in the diff to cover the new --merge behavior or the TTY handling in the install script. 📝 Found 2 issue(s). See inline comments for details. What to expectKimchi will analyze the changes in this pull request and post:
The review typically completes within a few minutes. This comment will be updated once the review is ready. Interact with Kimchi
ConfigurationReviews are configured by your organization admin. Powered by Kimchi — AI-powered code review by CAST AI |
![kimchi-review[bot]](https://avatars.githubusercontent.com/in/3223788?s=60&v=4){kind=small}
There was a problem hiding this comment.
📊 Review Score: 85/100 (overall code quality — 0 lowest, 100 highest)
⏱️ Estimated effort to review: 2/5 (1 = trivial, 5 = very complex)
🧪 Tests: no — No test files were modified or added in the diff to cover the new --merge behavior or the TTY handling in the install script.
📝 Found 2 issue(s). See inline comments for details.
| return fmt.Errorf("openclaw config set: %s: %w", string(out), err) | ||
| } | ||
|
|
||
| // Merge fallbacks so existing entries are preserved. |
There was a problem hiding this comment.
The error return from json.Marshal is discarded when marshaling fallbacksJSON. While unlikely to fail for a simple string slice, if it does return an error, fallbacksJSON could be nil and string(nil) would result in an empty string being passed to the openclaw config set command, potentially clearing the user's fallback configuration instead of merging.
💡 Suggestion: Check the error from json.Marshal: fallbacksJSON, err := json.Marshal(...); if err != nil { return fmt.Errorf("marshal fallbacks: %w", err) }
| modelMap = make(map[string]any) | ||
| } | ||
| modelMap["primary"] = providerName + "/" + reg.Main().Slug | ||
| // Merge fallbacks so existing entries are preserved. |
There was a problem hiding this comment.
ℹ️🔧 Maintainability
The type assertion existingFallbacks, _ := modelMap["fallbacks"].([]any) discards the boolean ok-value. If the existing config has fallbacks stored as a different type (e.g., []string instead of []any from JSON decoding), the assertion fails silently and existingFallbacks becomes nil, causing the append to effectively replace rather than merge the fallbacks.
💡 Suggestion: Consider logging a warning if the type assertion fails, or use reflection to handle type conversion safely if the source data format is uncertain.
|
Consolidated into #50 — the openclaw merge fix is now part of the same branch. |
1 participant
Kimchi Summary
What changed
Fixes OpenClaw configuration merging to preserve existing user settings (temperature, max_tokens, custom fallbacks) instead of overwriting them. Also improves the install script to handle non-interactive environments (e.g.,
curl | bash).Why
Previously, running
kimchiwould wipe out existing OpenClaw configuration values liketemperatureormax_tokenswhen updating model settings. This change ensures user customizations are preserved during configuration updates.Key changes
internal/tools/openclaw.go:--mergeflag toopenclaw config setCLI commands foragents.defaults.model.fallbacksandagents.defaults.models(both sequential and batch paths)writeOpenClawViaCLIto runfallbacksandmodelsas separate--mergeoperations after the main--batch-jsoncallwriteOpenClawDirectto merge into existingmodelandmodelsmaps rather than replacing them, preserving existing fallbacks and catalog entriesscripts/install.sh:/dev/ttywhen stdin is not a terminal, or prompts user to manually runkimchiif no TTY is availableImpact
curl ... | bash) without hanging or failing to launch Kimchi