fix: add catalog validation to `models set` command by carrotRakko · Pull Request #18129 · openclaw/openclaw

carrotRakko · 2026-02-16T14:41:38Z

Summary

Problem: models set accepts any string as model ID — typos silently persist in config and fail at runtime with "Unknown model"
Why it matters: Silent misconfiguration is hard to diagnose; empty {} entries bypass provider routing constraints configured for other models
What changed: Added catalog validation (rejects unknown model IDs) and a warning when adding models with no existing config entry
What did NOT change (scope boundary): No changes to /model chat command, models list, or the allowlist/routing logic itself

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes [Bug]: models set CLI skips catalog validation and overwrites model routing config with empty entry #17183

User-visible / Behavior Changes

openclaw models set <invalid-model> now exits with an error: Unknown model: <id> (previously succeeded silently)
openclaw models set <valid-model-without-config> now prints a warning before applying (previously silent)
When the catalog is empty (initial setup before first gateway connection), validation is skipped to avoid blocking setup

Security Impact (required)

New permissions/capabilities? No
Secrets/tokens handling changed? No
New/changed network calls? No
Command/tool execution surface changed? No
Data access scope changed? No

Repro + Verification

Environment

OS: Amazon Linux 2023 (aarch64)
Runtime/container: Node.js v22, npm global install
Model/provider: OpenRouter
Integration/channel (if any): Slack (socket mode)
Relevant config (redacted): agents.defaults.models with provider routing entries

Steps

openclaw models set openrouter/nonexistent/fake-model-v99
Observe error message
openclaw models set openrouter/openai/gpt-4o (valid model not in config)
Observe warning + successful update

Expected

Step 1: Error with "Unknown model" message
Step 3: Warning about empty config, then success

Actual

Before fix: Both steps succeed silently
After fix: Matches expected behavior

Evidence

Trace/log snippets

Before:

$ openclaw models set openrouter/typo/nonexistent
Config updated.
Default model: openrouter/typo/nonexistent

After:

$ openclaw models set openrouter/typo/nonexistent
Error: Unknown model: openrouter/typo/nonexistent
Model not found in catalog. Run "openclaw models list" to see available models.

Human Verification (required)

Verified scenarios: Typo rejection, valid model switch, new model addition warning
Edge cases checked: Empty catalog (initial setup) — validation is skipped
What you did not verify: Interaction with --force or --skip-validation flags (none exist)

Compatibility / Migration

Backward compatible? Yes — existing valid configurations are unaffected
Config/env changes? No
Migration needed? No

Failure Recovery (if this breaks)

How to disable/revert this change quickly: Revert this single commit
Files/config to restore: src/commands/models/set.ts
Known bad symptoms reviewers should watch for: False rejections of valid model IDs (would indicate catalog loading failure)

Risks and Mitigations

Risk: Catalog loading may fail in offline/air-gapped environments, causing all models set to be rejected
- Mitigation: Catalog is skipped when empty (length === 0), which covers initial setup and environments where the gateway hasn't been contacted

✍️ Author: Claude Code with @carrotRakko (AI-written, human-approved)

Greptile Summary

Added validation to reject invalid model IDs in models set command by checking against the catalog loaded from loadModelCatalog. The validation is skipped when the catalog is empty to allow initial setup scenarios. The implementation also warns when adding models that don't have existing config entries.

The change prevents typos from being silently persisted in config by validating model IDs against the catalog before applying updates. The warning for new entries helps users understand when models are being added without explicit provider routing configuration.

Confidence Score: 4/5

This PR is safe to merge with only minor concerns about test coverage
The implementation is straightforward and follows existing patterns. The validation logic is sound and includes proper edge case handling (empty catalog). However, the new validation logic lacks test coverage, which prevents a score of 5.
Consider adding tests that mock loadModelCatalog to verify the validation behavior in src/commands/models.set.e2e.test.ts

_{Last reviewed commit: 11aab35}

@carrotRakko

`models set` accepts any syntactically valid model ID without checking the catalog, allowing typos to silently persist in config and fail at runtime. It also unconditionally adds an empty `{}` entry to `agents.defaults.models`, bypassing any provider routing constraints. This commit: - Validates the model ID against the catalog (skipped when catalog is empty during initial setup) - Warns when a new entry is added with empty config (no provider routing) Closes openclaw#17183 ✍️ Author: Claude Code with @carrotRakko (AI-written, human-approved)

greptile-apps

_{1 file reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-16T14:45:02Z

src/commands/models/set.ts

+  }
+
+  // 3. Track whether this is a new entry
+  const isNewEntry = !cfg.agents?.defaults?.models?.[key];


The isNewEntry check uses the first config snapshot (cfg), but updateConfig reads the config again internally. If the config file changes between these two reads, isNewEntry could be incorrect. While unlikely in typical CLI usage, this could be avoided by checking isNewEntry after the update using the updated config returned from updateConfig.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/commands/models/set.ts Line: 31:31 Comment: The `isNewEntry` check uses the first config snapshot (`cfg`), but `updateConfig` reads the config again internally. If the config file changes between these two reads, `isNewEntry` could be incorrect. While unlikely in typical CLI usage, this could be avoided by checking `isNewEntry` after the update using the `updated` config returned from `updateConfig`. How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-16T14:45:04Z

src/commands/models/set.ts

+  const resolved = resolveModelTarget({ raw: modelRaw, cfg });
+  const key = `${resolved.provider}/${resolved.model}`;


Resolved model target at line 16, which is then resolved again inside applyDefaultModelPrimaryUpdate (shared.ts:177)

Prompt To Fix With AI

This is a comment left during a code review. Path: src/commands/models/set.ts Line: 16:17 Comment: Resolved model target at line 16, which is then resolved again inside `applyDefaultModelPrimaryUpdate` (shared.ts:177) How can I resolve this? If you propose a fix, please make it concise.

sebslight · 2026-02-17T14:16:10Z

Reverted in 676cf08 via #19194.

This PR was an accidental merge; reverting to restore previous behavior.

carrotRakko · 2026-02-18T06:25:58Z

@steipete @sebslight Could you clarify what "accidental merge" means here?

Was this a merge-button mistake (clicked merge by accident)?
Or was there a technical issue with the change itself?

The revert comment says "reverting to restore previous behavior" but doesn't explain what was wrong with the new behavior. If there's a specific concern, I'd like to address it and resubmit.

The underlying issue (#17183 — models set silently accepting invalid model IDs) still exists.

✍️ Author: Claude Code with @carrotRakko (AI-written, human-approved)

openclaw-barnacle bot added commands Command implementations size: XS labels Feb 16, 2026

greptile-apps bot reviewed Feb 16, 2026

View reviewed changes

steipete merged commit afd354c into openclaw:main Feb 16, 2026
26 checks passed

sebslight mentioned this pull request Feb 17, 2026

revert: fix models set catalog validation #19194

Merged

carrotRakko mentioned this pull request Feb 23, 2026

[Bug]: models set accepts invalid model IDs without catalog validation #24243

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: add catalog validation to `models set` command#18129

fix: add catalog validation to `models set` command#18129
steipete merged 1 commit intoopenclaw:mainfrom
delight-co:fix/models-set-catalog-validation

carrotRakko commented Feb 16, 2026 •

edited by greptile-apps bot

Loading

Uh oh!

greptile-apps bot left a comment

Uh oh!

greptile-apps bot Feb 16, 2026

Uh oh!

greptile-apps bot Feb 16, 2026

Uh oh!

Uh oh!

sebslight commented Feb 17, 2026

Uh oh!

carrotRakko commented Feb 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		const resolved = resolveModelTarget({ raw: modelRaw, cfg });
		const key = `${resolved.provider}/${resolved.model}`;

Uh oh!

Conversation

carrotRakko commented Feb 16, 2026 • edited by greptile-apps bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

User-visible / Behavior Changes

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual

Evidence

Human Verification (required)

Compatibility / Migration

Failure Recovery (if this breaks)

Risks and Mitigations

Greptile Summary

Confidence Score: 4/5

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sebslight commented Feb 17, 2026

Uh oh!

carrotRakko commented Feb 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

carrotRakko commented Feb 16, 2026 •

edited by greptile-apps bot

Loading