← Back to PRs

#6673: fix: preserve allowAny flag in createModelSelectionState for custom providers (#2144)

by tenor0 open 2026-02-01 23:01 View on GitHub →
## Summary Fix sub-agent model overrides being silently discarded for custom (non-catalog) providers like DeepSeek, OpenRouter, and other user-configured providers. **Fixes:** #2144, #2489, #3145 ## Problem When `sessions_spawn` is called with a `model` override pointing to a custom provider (e.g., `deepseek/deepseek-chat`): 1. `sessions.patch` correctly validates the model using `buildAllowedModelSet().allowAny` → sets `modelApplied: true` ✅ 2. But when the sub-agent actually runs, `createModelSelectionState` **discards the `allowAny` flag** and uses only `allowedModelKeys` (the built-in catalog) 3. Custom provider models aren't in the Pi SDK catalog → `allowedModelKeys.has(key)` returns `false` 4. **Check 1 (L322):** Silently resets the override back to the default model 5. **Check 2 (L349):** Refuses to apply the stored override 6. Sub-agent runs on the default Anthropic model despite `modelApplied: true` This only affects **custom providers** — built-in catalog models (Anthropic, OpenAI, Google) work fine because they're in `allowedModelKeys`. ## Root Cause `createModelSelectionState` in `src/auto-reply/reply/model-selection.ts` reads `allowedKeys` and `allowedCatalog` from `buildAllowedModelSet()` but **never reads `allowAny`**: ```typescript // BEFORE (bug): const allowed = buildAllowedModelSet({ cfg, catalog, defaultProvider, defaultModel }); allowedModelCatalog = allowed.allowedCatalog; allowedModelKeys = allowed.allowedKeys; // ^^^ allowed.allowAny is DISCARDED ``` Then two validation checks use only the key set: ```typescript // Check 1 (~L322): Resets override if not in catalog if (allowedModelKeys.size > 0 && !allowedModelKeys.has(key)) { applyModelOverrideToSessionEntry({ entry, selection: { ...default } }); // SILENT RESET! } // Check 2 (~L349): Applies override only if in catalog if (allowedModelKeys.size === 0 || allowedModelKeys.has(key)) { provider = candidateProvider; model = storedOverride.model; } ``` When `allowAny = true` (no explicit allowlist configured — the common case), `allowedModelKeys` still contains all catalog models, so `size > 0` is true, but custom provider models aren't in the set. ## Fix **4-line change** — capture `allowAny` and use it in both checks: ```typescript // AFTER (fix): const allowed = buildAllowedModelSet({ cfg, catalog, defaultProvider, defaultModel }); allowedModelCatalog = allowed.allowedCatalog; allowedModelKeys = allowed.allowedKeys; let allowAnyModel = allowed.allowAny; // ← NEW // Check 1: Don't reset if allowAny is true if (!allowAnyModel && allowedModelKeys.size > 0 && !allowedModelKeys.has(key)) { ... } // Check 2: Apply if allowAny is true if (allowAnyModel || allowedModelKeys.size === 0 || allowedModelKeys.has(key)) { ... } ``` This makes `createModelSelectionState` consistent with the `sessions.patch` path, which already uses `allowAny` correctly. ## Testing ### Unit Tests (4 new, all existing pass) | Test | Description | Status | |------|-------------|--------| | Custom provider preserved (DeepSeek) | Override `deepseek/deepseek-chat` not reset when `allowAny=true` | ✅ | | Custom provider preserved (OpenRouter) | Override `openrouter/google/gemini-2.5-flash` not reset | ✅ | | Built-in model still works | Anthropic model override unaffected (regression) | ✅ | | Allowlist still enforced | Explicit allowlist rejects unlisted models | ✅ | **Existing test suite:** 22 test files in `src/auto-reply/reply/`, all passing (0 regressions). ### Live Instance Validation Tested on a running Clawdbot `2026.1.24-3` instance (Ubuntu ARM64, Node 22) with DeepSeek configured as a custom provider: | Test Case | Model Requested | Model Used | Result | |-----------|----------------|------------|--------| | TC-01: Custom provider | `deepseek/deepseek-chat` | `deepseek/deepseek-chat` | ✅ Confirmed via API response | | TC-02: Built-in model | `anthropic/claude-sonnet-4-5` | `anthropic/claude-sonnet-4-5` | ✅ | | TC-03: Default (no override) | *(none)* | `anthropic/claude-sonnet-4-5` (config default) | ✅ | | TC-04: Invalid model | `fakeprovider/nonexistent-model-9000` | Falls back to instance default | ✅ Graceful degradation | **Before fix:** TC-01 silently fell back to `anthropic/claude-opus-4-5` despite `modelApplied: true`. **After fix:** TC-01 correctly routes to DeepSeek and receives a response from `deepseek-chat`. ### Verification Method Sub-agent transcript confirmed actual API calls: ```json { "api": "openai-completions", "provider": "deepseek", "model": "deepseek-chat", "stopReason": "stop" } ``` ## Impact - **Scope:** Only affects `createModelSelectionState` in model-selection.ts - **Risk:** Minimal — adds a boolean guard to two existing conditionals - **Backward compatible:** When `allowAny = false` (explicit allowlist configured), behavior is unchanged - **No breaking changes:** All 22 existing test files pass without modification ## Files Changed | File | Change | |------|--------| | `src/auto-reply/reply/model-selection.ts` | +2 lines (capture `allowAny`), ~2 lines modified (add guards) | | `src/auto-reply/reply/model-selection.allow-any-custom-provider.test.ts` | New file: 4 test cases | <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> This PR fixes model override handling for sub-agents when the override points to a custom (non-catalog) provider. `createModelSelectionState` now preserves and consults `allowAny` from `buildAllowedModelSet`, preventing overrides like `deepseek/deepseek-chat` from being silently reset to defaults when no explicit allowlist is configured. It also adds a focused Vitest suite that covers allowAny + custom providers, a built-in model regression case, and allowlist enforcement. The change aligns the runtime selection path (`createModelSelectionState` in `src/auto-reply/reply/model-selection.ts`) with the existing validation semantics used elsewhere (via `buildAllowedModelSet`), so “modelApplied” and actual model execution behavior stay consistent for user-configured providers. <h3>Confidence Score: 4/5</h3> - This PR is likely safe to merge; it’s a small behavioral fix with targeted tests, though the new tests could be tightened and local test execution wasn’t verified here. - The runtime change is minimal (threading `allowAny` into two existing conditionals) and matches the intended semantics already encoded in `buildAllowedModelSet`. Added tests cover the previously broken path and allowlist enforcement. Confidence isn’t 5/5 because I couldn’t run the tests in this environment (missing pnpm) and the tests currently don’t assert that the session override reset side effect doesn’t occur in allowAny cases. - src/auto-reply/reply/model-selection.allow-any-custom-provider.test.ts <!-- 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