Skip to content

Commit 1addafb

Browse files
committed
fix(access-requests): recheck rollout and retain public models
1 parent e1db271 commit 1addafb

6 files changed

Lines changed: 116 additions & 31 deletions

File tree

‎apps/sim/lib/permission-access-requests/catalog-registry.ts‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import {
2626
import { getBlockRegistry } from '@/blocks/registry'
2727
import { isHiddenUnder } from '@/blocks/visibility/context'
2828
import { CONNECTOR_META_REGISTRY } from '@/connectors/registry'
29-
import { DYNAMIC_MODEL_PROVIDERS, PROVIDER_DEFINITIONS } from '@/providers/models'
29+
import { getStaticProviderModels, PROVIDER_DEFINITIONS } from '@/providers/models'
3030
import { filterBlacklistedModels } from '@/providers/utils'
3131
import { getToolMetadata } from '@/tools/metadata'
3232

@@ -36,8 +36,6 @@ export interface AccessRequestCatalogContext {
3636
workspaceId: string | null
3737
}
3838

39-
const DYNAMIC_PROVIDERS: ReadonlySet<string> = new Set(DYNAMIC_MODEL_PROVIDERS)
40-
4139
function isProviderDeploymentAvailable(providerId: string): boolean {
4240
if (providerId === 'ollama') return !isHosted || isOllamaUrlConfigured()
4341
if (providerId === 'vllm') return Boolean(env.VLLM_BASE_URL?.trim())
@@ -128,10 +126,9 @@ export async function loadAccessRequestRegistryCatalog(
128126
continue
129127
}
130128
providers.push({ id: provider.id, label: provider.name })
131-
/** Dynamic arrays may contain private names populated by another credential's discovery. */
132-
if (targetKind === 'provider' || DYNAMIC_PROVIDERS.has(provider.id)) continue
129+
if (targetKind === 'provider') continue
133130
const availableModelIds = filterBlacklistedModels(
134-
provider.models
131+
getStaticProviderModels(provider.id)
135132
.filter((model) => model.sunset?.status !== 'deprecated')
136133
.map((model) => model.id)
137134
)

‎apps/sim/lib/permission-access-requests/catalog.test.ts‎

Lines changed: 44 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -42,25 +42,37 @@ vi.mock('@/lib/integrations/availability.server', () => ({
4242
}))
4343
vi.mock('@/providers/utils', () => ({ filterBlacklistedModels: mocks.filterModels }))
4444
vi.mock('@/tools/metadata', () => ({ getToolMetadata: mocks.toolMetadata }))
45-
vi.mock('@/providers/models', () => ({
46-
DYNAMIC_MODEL_PROVIDERS: ['ollama', 'vllm', 'litellm', 'openrouter'],
47-
PROVIDER_DEFINITIONS: {
48-
openai: {
49-
id: 'openai',
50-
name: 'OpenAI',
51-
models: [
52-
{ id: 'public-model' },
53-
{ id: 'blocked-model' },
54-
{ id: 'retired-model', sunset: { status: 'deprecated' } },
55-
],
45+
vi.mock('@/providers/models', () => {
46+
const publicModels = {
47+
openai: [
48+
{ id: 'public-model' },
49+
{ id: 'blocked-model' },
50+
{ id: 'retired-model', sunset: { status: 'deprecated' } },
51+
],
52+
fireworks: [{ id: 'fireworks/public-model' }],
53+
}
54+
return {
55+
getStaticProviderModels: (providerId: string) =>
56+
publicModels[providerId as keyof typeof publicModels] ?? [],
57+
PROVIDER_DEFINITIONS: {
58+
openai: { id: 'openai', name: 'OpenAI', models: publicModels.openai },
59+
anthropic: { id: 'anthropic', name: 'Anthropic', models: [{ id: 'anthropic-model' }] },
60+
ollama: { id: 'ollama', name: 'Ollama', models: [{ id: 'private-local' }] },
61+
vllm: { id: 'vllm', name: 'vLLM', models: [] },
62+
litellm: { id: 'litellm', name: 'LiteLLM', models: [] },
63+
openrouter: {
64+
id: 'openrouter',
65+
name: 'OpenRouter',
66+
models: [{ id: 'private-tenant-model' }],
67+
},
68+
fireworks: {
69+
id: 'fireworks',
70+
name: 'Fireworks',
71+
models: [...publicModels.fireworks, { id: 'fireworks/private-model' }],
72+
},
5673
},
57-
anthropic: { id: 'anthropic', name: 'Anthropic', models: [{ id: 'anthropic-model' }] },
58-
ollama: { id: 'ollama', name: 'Ollama', models: [{ id: 'private-local' }] },
59-
vllm: { id: 'vllm', name: 'vLLM', models: [] },
60-
litellm: { id: 'litellm', name: 'LiteLLM', models: [] },
61-
openrouter: { id: 'openrouter', name: 'OpenRouter', models: [{ id: 'private-tenant-model' }] },
62-
},
63-
}))
74+
}
75+
})
6476
vi.mock('@/connectors/registry', () => ({
6577
CONNECTOR_META_REGISTRY: {
6678
available: {
@@ -187,11 +199,23 @@ describe('access request catalog deployment ceilings', () => {
187199

188200
it('omits blacklisted/retired models, unconfigured endpoints, and private dynamic names', async () => {
189201
const catalog = await loadAccessRequestCatalog(context)
190-
expect([...catalog.providers.keys()]).toEqual(['openai', 'openrouter'])
191-
expect([...catalog.models.keys()]).toEqual(['public-model'])
202+
expect([...catalog.providers.keys()]).toEqual(['openai', 'openrouter', 'fireworks'])
203+
expect([...catalog.models.keys()]).toEqual(['public-model', 'fireworks/public-model'])
192204
expect([...catalog.knowledgeConnectors.keys()]).toEqual(['available', 'token', 'fallback'])
193205
})
194206

207+
it('keeps public models of dynamic providers requestable without exposing private names', async () => {
208+
const catalog = await loadAccessRequestCatalog(context, 'model')
209+
210+
expect(catalog.models.get('fireworks/public-model')).toEqual({
211+
id: 'fireworks/public-model',
212+
label: 'fireworks/public-model',
213+
providerId: 'fireworks',
214+
})
215+
expect(catalog.models.has('fireworks/private-model')).toBe(false)
216+
expect(catalog.models.has('private-tenant-model')).toBe(false)
217+
})
218+
195219
it('refuses ambiguous tool parent policies instead of choosing one silently', async () => {
196220
mocks.blocks.mockReturnValue({
197221
first: { type: 'first', name: 'First', tools: { access: ['shared_tool'] } },

‎apps/sim/lib/permission-access-requests/settings.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,31 @@ describe('permission access request settings', () => {
5050
await expect(isAccessRequestEnabled('organization-one')).resolves.toBe(true)
5151
})
5252

53+
it('rechecks rollout after an enabled admission snapshot', async () => {
54+
mockIsFeatureEnabled.mockResolvedValue(false)
55+
56+
await expect(isAccessRequestEnabled('organization-one', undefined, true)).resolves.toBe(false)
57+
58+
expect(mockIsFeatureEnabled).toHaveBeenCalledExactlyOnceWith('permission-access-requests')
59+
expect(dbChainMockFns.select).not.toHaveBeenCalled()
60+
})
61+
62+
it('keeps a disabled admission snapshot denied even if rollout is now enabled', async () => {
63+
await expect(isAccessRequestEnabled('organization-one', undefined, false)).resolves.toBe(false)
64+
65+
expect(mockIsFeatureEnabled).not.toHaveBeenCalled()
66+
expect(dbChainMockFns.select).not.toHaveBeenCalled()
67+
})
68+
69+
it('requires the current organization preference after an enabled admission snapshot', async () => {
70+
queueTableRows(organizationAccessRequestSettings, [{ allowRequests: false }])
71+
72+
await expect(isAccessRequestEnabled('organization-one', undefined, true)).resolves.toBe(false)
73+
74+
expect(mockIsFeatureEnabled).toHaveBeenCalledExactlyOnceWith('permission-access-requests')
75+
expect(dbChainMockFns.select).toHaveBeenCalledTimes(1)
76+
})
77+
5378
it('honors an organization opt-out while global rollout is active', async () => {
5479
queueTableRows(organizationAccessRequestSettings, [{ allowRequests: false }])
5580

‎apps/sim/lib/permission-access-requests/settings.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,9 @@ export async function readAccessRequestSettings(organizationId: string, executor
1818
export async function isAccessRequestEnabled(
1919
organizationId: string,
2020
executor: DbOrTx = db,
21-
globalEnabled?: boolean
21+
enabledAtAdmission?: boolean
2222
): Promise<boolean> {
23-
if (!(globalEnabled ?? (await isFeatureEnabled('permission-access-requests')))) return false
23+
if (enabledAtAdmission === false || !(await isFeatureEnabled('permission-access-requests')))
24+
return false
2425
return (await readAccessRequestSettings(organizationId, executor)).allowRequests
2526
}

‎apps/sim/providers/models.test.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
getModelsWithPromptCaching,
1212
getPromptCachingMinimumTokens,
1313
getProviderModels,
14+
getStaticProviderModels,
1415
getThinkingStreamVisibility,
1516
isCustomModelId,
1617
isKnownModelId,
@@ -632,6 +633,37 @@ describe('fireworks static catalog (the sim-auto pool)', () => {
632633
})
633634
})
634635

636+
describe('getStaticProviderModels', () => {
637+
it('retains public built-in models after private models are discovered', () => {
638+
const originalModels = PROVIDER_DEFINITIONS.fireworks.models
639+
const publicModels = getStaticProviderModels('fireworks')
640+
try {
641+
updateFireworksModels(['fireworks/private-test-model'])
642+
643+
expect(publicModels.length).toBeGreaterThan(0)
644+
expect(getProviderModels('fireworks')).toContain('fireworks/private-test-model')
645+
expect(getStaticProviderModels('fireworks')).toEqual(publicModels)
646+
} finally {
647+
PROVIDER_DEFINITIONS.fireworks.models = originalModels
648+
}
649+
})
650+
651+
it("excludes discovered names even when they match another provider's public model", () => {
652+
const originalModels = PROVIDER_DEFINITIONS.ollama.models
653+
try {
654+
updateOllamaModels(['private-local-model', 'fireworks/glm-5.2'])
655+
656+
expect(getStaticProviderModels('ollama')).toEqual([])
657+
} finally {
658+
PROVIDER_DEFINITIONS.ollama.models = originalModels
659+
}
660+
})
661+
662+
it('returns no models for an unknown provider', () => {
663+
expect(getStaticProviderModels('unknown-provider')).toEqual([])
664+
})
665+
})
666+
635667
describe('isModelDeprecated', () => {
636668
it('returns true for a catalogued deprecated model (case-insensitive)', () => {
637669
const id = firstDeprecatedModelId()

‎apps/sim/providers/models.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4989,9 +4989,8 @@ interface ModelCatalogEntry {
49894989

49904990
/**
49914991
* Lowercased model ID → catalog position metadata, built once from the static
4992-
* provider catalog. Dynamic providers contribute nothing here because their model
4993-
* lists are populated at runtime (not at module load), and only catalog models are
4994-
* ever reordered by release date.
4992+
* provider catalog, including built-in models of dynamic providers. Models added
4993+
* by runtime discovery are excluded.
49954994
*/
49964995
const MODEL_CATALOG_INDEX: Map<string, ModelCatalogEntry> = new Map(
49974996
Object.entries(PROVIDER_DEFINITIONS).flatMap(([providerId, provider]) =>
@@ -5009,6 +5008,13 @@ const MODEL_CATALOG_INDEX: Map<string, ModelCatalogEntry> = new Map(
50095008
)
50105009
)
50115010

5011+
/** Returns built-in public models, excluding names added by runtime discovery. */
5012+
export function getStaticProviderModels(providerId: string): ModelDefinition[] {
5013+
return (PROVIDER_DEFINITIONS[providerId]?.models ?? []).filter(
5014+
(model) => MODEL_CATALOG_INDEX.get(model.id.toLowerCase())?.providerId === providerId
5015+
)
5016+
}
5017+
50125018
/**
50135019
* Reorders model IDs so that, within each provider, newer models (by release date)
50145020
* come first — while preserving the caller's existing provider grouping order. The

0 commit comments

Comments
 (0)