← Back to PRs

#21963: fix(cli): models fallbacks add now includes primary model in allowlist

by ashiabbott open 2026-02-20 15:40 View on GitHub →
commands size: S
## Summary - **Problem:** `openclaw models fallbacks add <model>` on a fresh config (no existing `models` allowlist) creates an allowlist containing only the new fallback, silently omitting the configured primary model. - **Why it matters:** All sessions and cron jobs relying on the primary model stop working after this command — with no error or warning. - **What changed:** In `addFallbackCommand`, before writing the updated allowlist, we now ensure the primary model (if configured) is seeded into `nextModels` when it would otherwise be absent. - **What did NOT change:** Behaviour when the allowlist already contains the primary is untouched (idempotent). ## Change Type (select all) - [x] Bug fix ## Scope (select all touched areas) - [x] Gateway / orchestration ## Linked Issue/PR - Closes #BUG-469 ## User-visible / Behavior Changes `openclaw models fallbacks add <model>` on a fresh config now preserves the primary model in the generated allowlist. Previously it silently dropped it. ## 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 ### Steps 1. Config has a primary model but no `models` allowlist 2. Run `openclaw models fallbacks add openai/gpt-5.2` 3. Inspect config — `models` now contains both primary and fallback ### Expected Both primary and fallback appear in `models` allowlist. ### Actual (before fix) Only the fallback appeared; primary was silently dropped. ## Evidence - [x] Failing test/log before + passing after Three new unit tests: regression case, no-phantom-key guard, idempotent case. ## Human Verification (required) - Verified: all three unit test scenarios pass - Not verified: live integration run with a real gateway ## Compatibility / Migration - Backward compatible? Yes - Config/env changes? No - Migration needed? No ## Failure Recovery (if this breaks) Remove the 12-line primary-seeding block added to `fallbacks-shared.ts`. ## Risks and Mitigations - Risk: `params.key === 'model'` guard may be too narrow for per-agent key paths. - Mitigation: Scoped to default-agent path only; per-agent fix is a separate concern. <!-- greptile_comment --> <h3>Greptile Summary</h3> Fixes critical bug where adding the first fallback on fresh config creates allowlist containing only the fallback, silently blocking the primary model. The fix seeds the primary model into the allowlist when it would otherwise be absent. **Issue found:** The fix only applies when the key is "model" but the same bug exists for "imageModel". The image fallbacks command will skip the primary-seeding logic and have the same allowlist bug. **What works:** Test coverage is comprehensive for the model path with regression, no-phantom-key, and idempotent scenarios. The core fix logic is sound. <h3>Confidence Score: 3/5</h3> - Partially safe - fixes the reported bug for text models but leaves the same bug in imageModel path - The PR correctly fixes the allowlist bug for the model path with good test coverage, but the fix is incomplete because the imageModel path has the identical bug that remains unfixed (line 91 guards only for "model" not "imageModel") - Pay attention to `fallbacks-shared.ts:91-97` which needs to handle both model types <sub>Last reviewed commit: 422cd05</sub> <!-- greptile_other_comments_section --> <sub>(2/5) Greptile learns from your feedback when you react with thumbs up/down!</sub> <!-- /greptile_comment -->

Most Similar PRs