← Back to PRs

#16838: fix: include configured fallbacks in model allowlist

by taw0002 open 2026-02-15 05:21 View on GitHub →
docs channel: slack agents stale size: S
## Problem When `agents.defaults.models` is set (enabling the model allowlist), models listed in `agents.defaults.model.fallbacks` are silently dropped if they aren't also present as keys in `agents.defaults.models`. This causes `All models failed (1)` errors when the primary provider enters cooldown (rate limit) — the configured fallback is never attempted despite being explicitly configured. ### Reproduction 1. Set `agents.defaults.model.primary = 'anthropic/claude-opus-4-6'` 2. Set `agents.defaults.model.fallbacks = ['openai/gpt-5.2']` 3. Set `agents.defaults.models = { 'anthropic/claude-opus-4-6': {} }` (common for aliases/params) 4. Trigger Anthropic rate limit (cooldown) 5. Expected: OpenAI fallback is attempted 6. Actual: `All models failed (1): anthropic/claude-opus-4-6: Provider anthropic is in cooldown` ### Root Cause `buildConfiguredAllowlistKeys()` in `model-selection.ts` only includes models from the `agents.defaults.models` map as allowlist entries. Configured fallbacks not listed there are filtered out by `resolveFallbackCandidates()`. ### Fix `buildConfiguredAllowlistKeys()` now includes all models explicitly referenced in config: - `agents.defaults.model.primary` + `.fallbacks[]` - `agents.defaults.imageModel.primary` + `.fallbacks[]` - `agents.list[].model` and `agents.list[].subagents.model` ### Test Added e2e test that reproduces the exact scenario: primary provider in cooldown + allowlist enabled + fallback not in allowlist → verifies fallback is still attempted. <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR contains two distinct changes bundled together: **1. Model allowlist fix** (`model-selection.ts`, `model-fallback.e2e.test.ts`): Fixes a bug where configured fallback models were silently dropped when `agents.defaults.models` (the model allowlist) was present but didn't include the fallback entries. `buildConfiguredAllowlistKeys()` now also collects models referenced in `agents.defaults.model.primary/fallbacks`, `agents.defaults.imageModel`, and `agents.list[].model/subagents.model`. A well-structured e2e test reproduces the exact failure scenario. **2. Slack thread inheritance default change** (`provider.ts`, system-prompt, docs, tests): Changes `thread.inheritParent` default from `false` to `true`, making thread replies route to the parent session by default. System prompt updated to tell agents not to use reply tags by default, deferring to user-configured `replyToMode`. - The new Slack test (`"disables thread inheritance when inheritParent is false"`) has **missing imports** for `waitForSlackEvent`, `getSlackHandlers`, and `flush` — this test will fail at runtime with `ReferenceError`. Either add the missing imports or refactor to use the `runSlackMessageOnce` helper like the other tests in the file. <h3>Confidence Score: 3/5</h3> - The core model-selection fix is sound, but the Slack test has missing imports that will cause a runtime failure. - The model allowlist fix in `model-selection.ts` is well-implemented with defensive type handling and a solid e2e test. The Slack `inheritParent` default change is consistent across docs, schema help, provider code, and system prompt. However, the new Slack test ("disables thread inheritance when inheritParent is false") uses `waitForSlackEvent`, `getSlackHandlers`, and `flush` without importing them, which will cause a `ReferenceError` at test runtime. This is a clear syntax/import issue that needs to be fixed before merge. - `src/slack/monitor.tool-result.threads-top-level-replies-replytomode-is-all.test.ts` — missing imports for `waitForSlackEvent`, `getSlackHandlers`, and `flush` in the new test case. <sub>Last reviewed commit: d60e560</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs