Skip to content
This repository was archived by the owner on May 15, 2026. It is now read-only.

fix/openclaw config merge #53

Closed
castabar wants to merge 2 commits into from
Closed

fix/openclaw config merge #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4c787e6
fix(install.sh): reattach stdin to /dev/tty when piped
castabar May 5, 2026
5b38527
fix(openclaw): merge config instead of replacing
castabar May 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 41 additions & 14 deletions internal/tools/openclaw.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func writeOpenClawViaCLISequential(providerBlock map[string]any, modelsCatalog m
providerName + "/" + reg.Coding().Slug,
providerName + "/" + reg.Sub().Slug,
})
cmd = exec.Command("openclaw", "config", "set", "agents.defaults.model.fallbacks", string(fallbacksJSON))
cmd = exec.Command("openclaw", "config", "set", "--merge", "agents.defaults.model.fallbacks", string(fallbacksJSON))
if out, err := cmd.CombinedOutput(); err != nil {
return fmt.Errorf("openclaw config set agents.defaults.model.fallbacks: %s: %w", string(out), err)
}
Expand All @@ -139,7 +139,7 @@ func writeOpenClawViaCLISequential(providerBlock map[string]any, modelsCatalog m
if err != nil {
return fmt.Errorf("marshal models catalog: %w", err)
}
cmd = exec.Command("openclaw", "config", "set", "agents.defaults.models", string(modelsJSON))
cmd = exec.Command("openclaw", "config", "set", "--merge", "agents.defaults.models", string(modelsJSON))
if out, err := cmd.CombinedOutput(); err != nil {
return fmt.Errorf("openclaw config set agents.defaults.models: %s: %w", string(out), err)
}
Expand Down Expand Up @@ -177,15 +177,10 @@ func writeOpenClawViaCLI(apiKey string, reg *models.Registry) error {
// Check OpenClaw version to determine which approach to use
version := getOpenClawVersion()
if isBatchJSONSupported(version) {
// Use --batch-json to set all config in a single CLI call (~3s vs ~12s sequential).
// Use --batch-json for the fields that don't conflict with existing user config.
batchOps := []map[string]any{
{"path": "models.providers.kimchi", "value": providerBlock},
{"path": "agents.defaults.model.primary", "value": providerName + "/" + reg.Main().Slug},
{"path": "agents.defaults.model.fallbacks", "value": []string{
providerName + "/" + reg.Coding().Slug,
providerName + "/" + reg.Sub().Slug,
}},
{"path": "agents.defaults.models", "value": modelsCatalog},
}

batchJSON, err := json.Marshal(batchOps)
Expand All @@ -197,6 +192,26 @@ func writeOpenClawViaCLI(apiKey string, reg *models.Registry) error {
if out, err := cmd.CombinedOutput(); err != nil {
return fmt.Errorf("openclaw config set: %s: %w", string(out), err)
}

// Merge fallbacks so existing entries are preserved.
Copy link
Copy Markdown

@kimchi-review kimchi-review Bot May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️⚠️ Error Handling

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) }

fallbacksJSON, _ := json.Marshal([]string{
providerName + "/" + reg.Coding().Slug,
providerName + "/" + reg.Sub().Slug,
})
cmd = exec.Command("openclaw", "config", "set", "--merge", "agents.defaults.model.fallbacks", string(fallbacksJSON))
if out, err := cmd.CombinedOutput(); err != nil {
return fmt.Errorf("openclaw config set --merge agents.defaults.model.fallbacks: %s: %w", string(out), err)
}

// Merge models catalog so existing entries are preserved.
modelsJSON, err := json.Marshal(modelsCatalog)
if err != nil {
return fmt.Errorf("marshal models catalog: %w", err)
}
cmd = exec.Command("openclaw", "config", "set", "--merge", "agents.defaults.models", string(modelsJSON))
if out, err := cmd.CombinedOutput(); err != nil {
return fmt.Errorf("openclaw config set --merge agents.defaults.models: %s: %w", string(out), err)
}
} else {
// Fall back to sequential calls for older versions
if err := writeOpenClawViaCLISequential(providerBlock, modelsCatalog, reg); err != nil {
Expand Down Expand Up @@ -310,13 +325,25 @@ func writeOpenClawDirect(scope config.ConfigScope, apiKey string, reg *models.Re
if defaults == nil {
defaults = make(map[string]any)
}
defaults["model"] = map[string]any{
"primary": providerName + "/" + reg.Main().Slug,
"fallbacks": []any{providerName + "/" + reg.Coding().Slug, providerName + "/" + reg.Sub().Slug},
// Merge into model config to preserve existing keys like temperature, max_tokens.
modelMap, _ := defaults["model"].(map[string]any)
if modelMap == nil {
modelMap = make(map[string]any)
}
modelMap["primary"] = providerName + "/" + reg.Main().Slug
// Merge fallbacks so existing entries are preserved.
Copy link
Copy Markdown

@kimchi-review kimchi-review Bot May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️🔧 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.

existingFallbacks, _ := modelMap["fallbacks"].([]any)
modelMap["fallbacks"] = append(existingFallbacks,
providerName+"/"+reg.Coding().Slug,
providerName+"/"+reg.Sub().Slug,
)
defaults["model"] = modelMap

// Merge into the allowed models catalog, preserving existing entries.
modelsCatalog, _ := defaults["models"].(map[string]any)
if modelsCatalog == nil {
modelsCatalog = make(map[string]any)
}

// Add models to the allowed models catalog.
modelsCatalog := make(map[string]any)
for _, m := range reg.All() {
modelsCatalog[providerName+"/"+m.Slug] = map[string]any{"alias": m.DisplayName}
}
Expand Down
9 changes: 8 additions & 1 deletion scripts/install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,11 @@ echo -e "${GREEN}✓ Installed kimchi to ${INSTALL_PATH}${NC}"
echo ""
echo -e "${BLUE}Launching Kimchi...${NC}"
echo ""
exec "$INSTALL_PATH"
if [ -t 0 ]; then
exec "$INSTALL_PATH"
elif [ -c /dev/tty ]; then
exec "$INSTALL_PATH" < /dev/tty
else
echo ""
echo -e "${BLUE}Run 'kimchi' to get started.${NC}"
fi
Loading