feat: add two-layer AI model availability filtering#18170
Conversation
Implement admin-level and workspace-level controls for AI model availability: **Admin layer** (server-wide): - New config variables: AI_AUTO_ENABLE_NEW_MODELS, AI_DISABLED_MODEL_IDS, AI_ENABLED_MODEL_IDS - New setAdminAiModelEnabled mutation with model ID validation - New AI tab in admin panel with filter dropdown (unconfigured/deprecated models) **Workspace layer** (per-workspace): - New workspace fields: autoEnableNewAiModels, disabledAiModelIds, enabledAiModelIds, useRecommendedModels - "Use best models only" mode backed by isRecommended flag on model definitions - Separate Smart/Fast model selectors with "Best (...)" virtual options **Security**: - Both layers enforced at every backend execution point (workspace update, agent create/update, chat execution) - AdminPanelGuard protects all admin AI endpoints - Model ID validated against known AI_MODELS before config mutation Co-authored-by: Cursor <cursoragent@cursor.com>
Adopt useRecoilValueV2/useRecoilStateV2 from upstream Recoil-to-Jotai migration while keeping our AI model availability filtering changes. Co-authored-by: Cursor <cursoragent@cursor.com>
...twenty-server/src/engine/metadata-modules/ai/ai-models/services/ai-model-registry.service.ts
Show resolved
Hide resolved
...twenty-server/src/engine/metadata-modules/ai/ai-models/services/ai-model-registry.service.ts
Outdated
Show resolved
Hide resolved
Greptile SummaryThis PR implements a two-layer AI model availability filtering system: admin-level (server-wide) controls via config variables and a new Admin AI panel, and workspace-level controls with "Use best models only" mode and custom whitelist/blacklist. Both layers are enforced at every backend execution point (workspace update, agent create/update, chat execution).
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User Action] --> B{Action Type}
B -->|Set Smart/Fast Model| C[WorkspaceService.updateWorkspace]
B -->|Create/Update Agent| D[AgentResolver]
B -->|Chat Stream| E[AgentChatController]
B -->|Chat Execute| F[ChatExecutionService]
B -->|Agent Workflow| G[AgentAsyncExecutorService]
C --> H{Admin Layer Check}
D --> H
E --> H
F --> H
G --> H
H -->|isModelAdminAllowed| I{Allowed?}
I -->|No| J[Reject: Disabled by admin]
I -->|Yes| K{Workspace Layer Check}
K -->|isModelAllowedByWorkspace| L{useRecommendedModels?}
L -->|Yes| M{Model isRecommended?}
L -->|No| N{autoEnableNewAiModels?}
M -->|Yes| O[Allow]
M -->|No| P[Reject: Not in workspace]
N -->|Yes - Blacklist mode| Q{In disabledAiModelIds?}
N -->|No - Whitelist mode| R{In enabledAiModelIds?}
Q -->|No| O
Q -->|Yes| P
R -->|Yes| O
R -->|No| P
O --> S[Execute Request]
Last reviewed commit: 57c68a0 |
| for (const modelId of modelsToValidate) { | ||
| const autoEnable = this.twentyConfigService.get( | ||
| 'AI_AUTO_ENABLE_NEW_MODELS', | ||
| ); | ||
| const disabledIds = parseCommaList( | ||
| this.twentyConfigService.get('AI_DISABLED_MODEL_IDS'), | ||
| ); | ||
| const enabledIds = parseCommaList( | ||
| this.twentyConfigService.get('AI_ENABLED_MODEL_IDS'), | ||
| ); | ||
| const isAdminAllowed = autoEnable | ||
| ? !disabledIds.has(modelId) | ||
| : enabledIds.has(modelId); |
There was a problem hiding this comment.
Missing virtual model bypass in admin check
The inline admin-level validation here does not bypass for virtual model IDs (DEFAULT_FAST_MODEL / DEFAULT_SMART_MODEL). In contrast, AiModelRegistryService.isModelAdminAllowed() correctly returns true early for these IDs (line 376-378 of ai-model-registry.service.ts).
When AI_AUTO_ENABLE_NEW_MODELS is false, a user attempting to set their Smart or Fast model to the "Best (...)" virtual option ('default-smart-model' or 'default-fast-model') will be rejected because the virtual ID will never appear in AI_ENABLED_MODEL_IDS.
Additionally, the config reads (twentyConfigService.get(...)) are unnecessarily inside the for loop — they are invariant between iterations and should be hoisted above it.
Consider reusing this.aiModelRegistryService.isModelAdminAllowed(modelId) instead of duplicating the logic:
| for (const modelId of modelsToValidate) { | |
| const autoEnable = this.twentyConfigService.get( | |
| 'AI_AUTO_ENABLE_NEW_MODELS', | |
| ); | |
| const disabledIds = parseCommaList( | |
| this.twentyConfigService.get('AI_DISABLED_MODEL_IDS'), | |
| ); | |
| const enabledIds = parseCommaList( | |
| this.twentyConfigService.get('AI_ENABLED_MODEL_IDS'), | |
| ); | |
| const isAdminAllowed = autoEnable | |
| ? !disabledIds.has(modelId) | |
| : enabledIds.has(modelId); | |
| for (const modelId of modelsToValidate) { | |
| if (!this.aiModelRegistryService.isModelAdminAllowed(modelId)) { | |
| throw new WorkspaceException( | |
| 'Selected model has been disabled by the administrator', | |
| WorkspaceExceptionCode.ENVIRONMENT_VAR_NOT_ENABLED, | |
| ); | |
| } | |
| if (!isModelAllowedByWorkspace(modelId, effectiveWorkspace)) { | |
| throw new WorkspaceException( | |
| 'Selected model is not available in this workspace', | |
| WorkspaceExceptionCode.ENVIRONMENT_VAR_NOT_ENABLED, | |
| ); | |
| } | |
| } |
This requires injecting AiModelRegistryService into WorkspaceService, but it eliminates the duplicated logic and the missing virtual model bypass.
| const workspace = await this.workspaceRepository.findOneBy({ | ||
| id: agent.workspaceId, | ||
| }); | ||
|
|
||
| if (workspace && !isModelAllowedByWorkspace(agentModelId, workspace)) { | ||
| throw new AgentException( | ||
| 'The selected model is not available in this workspace.', | ||
| AgentExceptionCode.AGENT_EXECUTION_FAILED, | ||
| ); | ||
| } |
There was a problem hiding this comment.
DB query per agent execution for workspace check
Every agent execution now does a findOneBy database query to fetch the workspace entity solely for the model availability check. This adds latency to every agent execution. Consider passing the workspace from the caller context if it's already available, or caching the workspace availability settings.
Context Used: Context from dashboard - Ideally, we should use the cached metadata when possible instead of querying the DB, especially for ... (source)
| @@ -73,6 +75,22 @@ export class AgentChatController { | |||
| ); | |||
| } | |||
There was a problem hiding this comment.
Unused result from getAdminFilteredModels()
getAdminFilteredModels() fetches and filters all available models, but only the .length is checked. The filtered list itself is never used. Since isModelAdminAllowed(resolvedModelId) is checked separately below (line 80), consider replacing this with a simpler availability check (e.g., getAvailableModels().length === 0) to avoid unnecessary work, or remove it entirely if the per-model check on line 80 is sufficient.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
6 issues found across 38 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/twenty-server/src/engine/metadata-modules/ai/ai-models/services/ai-model-registry.service.ts">
<violation number="1" location="packages/twenty-server/src/engine/metadata-modules/ai/ai-models/services/ai-model-registry.service.ts:376">
P2: Default smart/fast models are always treated as admin-allowed, so admin allow/deny lists cannot disable them. This contradicts the admin filtering behavior and lets disabled defaults still be selectable/used.</violation>
</file>
<file name="packages/twenty-front/src/pages/settings/ai/utils/getModelFamilyProperties.ts">
<violation number="1" location="packages/twenty-front/src/pages/settings/ai/utils/getModelFamilyProperties.ts:19">
P2: Avoid silent fallbacks when indexing model-family config. This makes missing mappings invisible and conflicts with the preference for explicit enum-to-config mappings.
(Based on your team's feedback about using strongly typed enum mappings and avoiding silent fallbacks for unknown model families.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/twenty-server/src/engine/core-modules/workspace/services/workspace.service.ts">
<violation number="1" location="packages/twenty-server/src/engine/core-modules/workspace/services/workspace.service.ts:227">
P2: Model availability settings can be updated without revalidating the current smart/fast models. This allows a workspace to enable "recommended-only" (or new allow/deny lists) while keeping previously disallowed models, undermining the enforcement described in this PR. Consider validating the effective smart/fast models whenever availability settings change, even if the model IDs themselves are unchanged.</violation>
<violation number="2" location="packages/twenty-server/src/engine/core-modules/workspace/services/workspace.service.ts:264">
P2: The new workspace update validation never verifies that `smartModel`/`fastModel` exist in the model registry. With `autoEnableNewAiModels` enabled, any unknown model ID not in the disabled list will pass `isModelAllowedByWorkspace`, allowing invalid model IDs to be stored. Consider validating against `AI_MODELS`/registry before checking allow lists.</violation>
</file>
<file name="packages/twenty-server/src/engine/metadata-modules/ai/ai-chat/services/chat-execution.service.ts">
<violation number="1" location="packages/twenty-server/src/engine/metadata-modules/ai/ai-chat/services/chat-execution.service.ts:142">
P2: This uses API_KEY_NOT_CONFIGURED for a model-availability failure, which produces a misleading userFriendlyMessage ("API key is not configured") alongside the error about the selected model. Use a generic code (e.g., AGENT_EXECUTION_FAILED) so the user-friendly message doesn’t contradict the real cause.
(Based on your team's feedback about keeping userFriendlyMessage generic and non-conflicting with the main error.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/twenty-server/src/engine/metadata-modules/ai/ai-agent-execution/services/agent-async-executor.service.ts">
<violation number="1" location="packages/twenty-server/src/engine/metadata-modules/ai/ai-agent-execution/services/agent-async-executor.service.ts:116">
P1: Admin/workspace filtering is bypassed for DEFAULT_SMART_MODEL/DEFAULT_FAST_MODEL because the checks use agent.modelId directly, and the helper functions always allow those virtual IDs. Resolve the effective model ID first and validate that instead so disabled models can’t be used via the default option.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
...r/src/engine/metadata-modules/ai/ai-agent-execution/services/agent-async-executor.service.ts
Outdated
Show resolved
Hide resolved
...twenty-server/src/engine/metadata-modules/ai/ai-models/services/ai-model-registry.service.ts
Show resolved
Hide resolved
packages/twenty-front/src/pages/settings/ai/utils/getModelFamilyProperties.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/core-modules/workspace/services/workspace.service.ts
Show resolved
Hide resolved
...ages/twenty-server/src/engine/metadata-modules/ai/ai-chat/services/chat-execution.service.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/core-modules/workspace/services/workspace.service.ts
Outdated
Show resolved
Hide resolved
- WorkspaceService: use AiModelRegistryService.isModelAdminAllowed() instead of duplicating admin check logic (fixes missing virtual model bypass) - WorkspaceService: also validate current smart/fast models when availability settings change (e.g., enabling recommended-only mode) - chat-execution.service: use AGENT_EXECUTION_FAILED instead of misleading API_KEY_NOT_CONFIGURED for model availability errors - agent-chat.controller: replace unused getAdminFilteredModels() with simpler getAvailableModels() check, fix exception codes Co-authored-by: Cursor <cursoragent@cursor.com>
These are settings pages with ~20 items max. The memoization wrappers added dependency array complexity without meaningful performance benefit, and some handlers (with currentWorkspace in deps) were recreating on every workspace change anyway, masking potential stale closure issues. Co-authored-by: Cursor <cursoragent@cursor.com>
...ages/twenty-server/src/engine/metadata-modules/ai/ai-chat/services/chat-execution.service.ts
Outdated
Show resolved
Hide resolved
- Change AI_DISABLED_MODEL_IDS and AI_ENABLED_MODEL_IDS config variables from STRING to ARRAY type, so the config framework handles comma parsing natively. This eliminates the custom parseCommaList utility entirely. - Split getModelFamilyProperties.ts into getModelIcon.ts and getModelProviderLabel.ts (one export per file). - Update all consumers accordingly. Co-authored-by: Cursor <cursoragent@cursor.com>
...twenty-server/src/engine/metadata-modules/ai/ai-models/services/ai-model-registry.service.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
2 issues found across 9 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/twenty-front/src/pages/settings/ai/utils/getModelIcon.ts">
<violation number="1" location="packages/twenty-front/src/pages/settings/ai/utils/getModelIcon.ts:12">
P2: Avoid normalizing free-form strings and silently falling back for unknown model families. This makes missing config keys invisible and bypasses type safety; prefer a strongly typed key (enum/union) with explicit mapping or explicit error handling for unknown values.
(Based on your team's feedback about using strongly typed enums and avoiding silent fallbacks for model family keys.) [FEEDBACK_USED]</violation>
</file>
<file name="packages/twenty-front/src/pages/settings/ai/utils/getModelProviderLabel.ts">
<violation number="1" location="packages/twenty-front/src/pages/settings/ai/utils/getModelProviderLabel.ts:10">
P2: Normalize-and-fallback lookup hides missing model-family mappings. Prefer a strongly typed key (e.g., `keyof typeof MODEL_FAMILY_CONFIG`) and explicit handling for unknown values instead of lowercasing and returning the raw string.
(Based on your team's feedback about strongly typed model family keys and avoiding silent fallbacks.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| return MODEL_FAMILY_CONFIG.FALLBACK.Icon; | ||
| } | ||
|
|
||
| const key = modelFamily.toLowerCase(); |
There was a problem hiding this comment.
P2: Avoid normalizing free-form strings and silently falling back for unknown model families. This makes missing config keys invisible and bypasses type safety; prefer a strongly typed key (enum/union) with explicit mapping or explicit error handling for unknown values.
(Based on your team's feedback about using strongly typed enums and avoiding silent fallbacks for model family keys.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-front/src/pages/settings/ai/utils/getModelIcon.ts, line 12:
<comment>Avoid normalizing free-form strings and silently falling back for unknown model families. This makes missing config keys invisible and bypasses type safety; prefer a strongly typed key (enum/union) with explicit mapping or explicit error handling for unknown values.
(Based on your team's feedback about using strongly typed enums and avoiding silent fallbacks for model family keys.) </comment>
<file context>
@@ -0,0 +1,15 @@
+ return MODEL_FAMILY_CONFIG.FALLBACK.Icon;
+ }
+
+ const key = modelFamily.toLowerCase();
+
+ return MODEL_FAMILY_CONFIG[key]?.Icon ?? MODEL_FAMILY_CONFIG.FALLBACK.Icon;
</file context>
| return ''; | ||
| } | ||
|
|
||
| const key = modelFamily.toLowerCase(); |
There was a problem hiding this comment.
P2: Normalize-and-fallback lookup hides missing model-family mappings. Prefer a strongly typed key (e.g., keyof typeof MODEL_FAMILY_CONFIG) and explicit handling for unknown values instead of lowercasing and returning the raw string.
(Based on your team's feedback about strongly typed model family keys and avoiding silent fallbacks.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-front/src/pages/settings/ai/utils/getModelProviderLabel.ts, line 10:
<comment>Normalize-and-fallback lookup hides missing model-family mappings. Prefer a strongly typed key (e.g., `keyof typeof MODEL_FAMILY_CONFIG`) and explicit handling for unknown values instead of lowercasing and returning the raw string.
(Based on your team's feedback about strongly typed model family keys and avoiding silent fallbacks.) </comment>
<file context>
@@ -0,0 +1,13 @@
+ return '';
+ }
+
+ const key = modelFamily.toLowerCase();
+
+ return MODEL_FAMILY_CONFIG[key]?.label ?? modelFamily;
</file context>
|
Hey @FelixMalfait! After you've done the QA of your Pull Request, you can mark it as done here. Thank you! |
Summary
AI_AUTO_ENABLE_NEW_MODELS,AI_DISABLED_MODEL_IDS,AI_ENABLED_MODEL_IDSconfig variables). DedicatedsetAdminAiModelEnabledmutation replaces frontend config-variable manipulation. Filter dropdown to show/hide unconfigured and deprecated models.isRecommendedflag), or custom whitelist/blacklist. Separate Smart/Fast model selectors with "Best (...)" virtual options.AdminPanelGuard.Changes
Backend (
twenty-server)AiModelRegistryService:getAllModelsWithStatus(),setModelAdminEnabled()with model ID validation,isModelAdminAllowed()AdminPanelResolver:getAdminAiModelsquery,setAdminAiModelEnabledmutationWorkspaceEntity: new fields (autoEnableNewAiModels,disabledAiModelIds,enabledAiModelIds,useRecommendedModels)WorkspaceService: model validation onsmartModel/fastModelupdatesAgentResolver: model availability checks on create/updateisModelAllowedByWorkspacecentralized utilityisRecommendedflag on model definitionsFrontend (
twenty-front)SettingsAdminAIcomponent with search, filter dropdown (unconfigured/deprecated), and model toggle cardsuseWorkspaceAiModelAvailabilityhook for workspace-level filteringSettingsAIModelsTabredesigned: merged sections, "Use best models only" toggle, conditional available models listgetModelIcon/getModelProviderLabelshared utilities with GraphQL enum casing normalizationTest plan
Made with Cursor