diff --git a/.agents/skills/add-feature-flag/SKILL.md b/.agents/skills/add-feature-flag/SKILL.md index a741530f412..bb415d8585f 100644 --- a/.agents/skills/add-feature-flag/SKILL.md +++ b/.agents/skills/add-feature-flag/SKILL.md @@ -1,16 +1,16 @@ --- name: add-feature-flag -description: Add a runtime feature flag (AppConfig-backed on prod, secret fallback off-prod), global by default or optionally gated by org id, user id, or platform admin +description: Add a runtime feature flag (AppConfig-backed on prod, secret fallback off-prod), global by default or optionally gated by workspace id, org id, user id, or platform admin argument-hint: --- # Add Feature Flag Skill -You add a **runtime feature flag** to Sim that can change on prod with no redeploy (AWS AppConfig). Prefer a global on/off flag unless the rollout actually needs per-organization, per-user, or platform-admin targeting. When AppConfig isn't the source of truth, the flag falls back to a single **secret** (on/off only). +You add a **runtime feature flag** to Sim that can change on prod with no redeploy (AWS AppConfig). Prefer a global on/off flag unless the rollout actually needs per-workspace, per-organization, per-user, or platform-admin targeting. When AppConfig isn't the source of truth, the flag falls back to a single **secret** (on/off only). ## When to use this vs `env-flags.ts` -- **Feature flag** (`@/lib/core/config/feature-flags.ts`): runtime global on/off by default, optionally scoped by `userId`/`orgId`/admin. This skill. +- **Feature flag** (`@/lib/core/config/feature-flags.ts`): runtime global on/off by default, optionally scoped by `workspaceId`/`userId`/`orgId`/admin. This skill. - **Env flag** (`@/lib/core/config/env-flags.ts`): deploy-time capability/environment detection (`isProd`, `isHosted`, `isBillingEnabled`). A module-load boolean. **Do not add gated flags here.** If the user wants a fixed per-deployment toggle, send them to `env-flags.ts` instead. @@ -21,10 +21,11 @@ A flag's **gating rule lives only in the hosted AppConfig document**. It is ON f ```ts interface FeatureFlagRule { - enabled?: boolean // global default for everyone - orgIds?: string[] // allowlisted organization ids - userIds?: string[] // allowlisted user ids - adminEnabled?: boolean // platform admins (user.role === 'admin') + enabled?: boolean // global default for everyone + workspaceIds?: string[] // allowlisted workspace ids + orgIds?: string[] // allowlisted organization ids + userIds?: string[] // allowlisted user ids + adminEnabled?: boolean // platform admins (user.role === 'admin') } ``` @@ -34,10 +35,10 @@ Critically, **none of this is expressible in code** — gating (especially `admi 1. **Confirm the granularity before editing code.** If the user has not already specified it, stop and ask: - > Should `` be a global on/off flag (recommended), or does it need rollout targeting by organization, user, and/or platform admin? + > Should `` be a global on/off flag (recommended), or does it need rollout targeting by workspace, organization, user, and/or platform admin? - - Recommend **global**. Do not infer scoped gating merely because the call site already has a user or organization id. - - If the user chooses scoped gating but does not name the dimensions, ask which of organization, user, and platform admin it needs. Wire only the selected dimensions. + - Recommend **global**. Do not infer scoped gating merely because the call site already has a workspace, user, or organization id. + - If the user chooses scoped gating but does not name the dimensions, ask which of workspace, organization, user, and platform admin it needs. Wire only the selected dimensions. - If the user wants a fixed per-deployment toggle rather than a runtime AppConfig flag, use `env-flags.ts` instead. 2. **Define the flag.** Add one entry to the `FEATURE_FLAGS` registry in `apps/sim/lib/core/config/feature-flags.ts`. Each entry is the flag's whole definition — name (kebab-case key), `description`, and the `fallback` secret consulted when AppConfig isn't the source of truth (truthy ⇒ on globally): @@ -51,7 +52,7 @@ Critically, **none of this is expressible in code** — gating (especially `admi } ``` - `fallback` is the env/secret key (typed as `keyof typeof env`), so add `` to `apps/sim/lib/core/config/env.ts` first (and the deployment's secret store) — it won't typecheck otherwise. Do **not** add org/user/admin defaults here — that gating exists only in AppConfig. Adding the entry makes `` a valid `FeatureFlagName`. + `fallback` is the env/secret key (typed as `keyof typeof env`), so add `` to `apps/sim/lib/core/config/env.ts` first (and the deployment's secret store) — it won't typecheck otherwise. Do **not** add workspace/org/user/admin defaults here — that gating exists only in AppConfig. Adding the entry makes `` a valid `FeatureFlagName`. 3. **Gate the call site at the chosen granularity.** For the recommended global mode, pass no context: @@ -70,17 +71,17 @@ Critically, **none of this is expressible in code** — gating (especially `admi ```ts import { isFeatureEnabled } from '@/lib/core/config/feature-flags' - if (await isFeatureEnabled('', { userId, orgId })) { + if (await isFeatureEnabled('', { workspaceId, userId, orgId })) { // gated behavior } ``` - - Organization targeting uses `orgId`; user and platform-admin targeting require `userId`. + - Workspace targeting uses `workspaceId`; organization targeting uses `orgId`; user and platform-admin targeting require `userId`. - Missing ids are fine — a clause with no matching id is skipped; with no `userId`, the admin clause resolves to `false` without a DB read. - Admin routes that already know the caller is an admin may pass `{ userId, isAdmin: true }` to skip the role lookup. - **Client/UI flags:** resolve server-side (in a server component, route, or loader) and pass the boolean down as a prop. There is no client AppConfig. -4. **(Prod) configure in AppConfig.** The infra `feature-flags` profile schema is permissive, so a new flag needs **no infra change**. Operators add the flag to the hosted `feature-flags` document using `enabled` for global rollout or only the selected `orgIds`/`userIds`/`adminEnabled` clauses for scoped rollout, then start a `sim--fast` deployment (see the AppConfig runbook in the infra README — same flow as `access-control`). The fallback secret only applies when AppConfig is disabled. +4. **(Prod) configure in AppConfig.** The infra `feature-flags` profile schema is permissive, so a new flag needs **no infra change**. Operators add the flag to the hosted `feature-flags` document using `enabled` for global rollout or only the selected `workspaceIds`/`orgIds`/`userIds`/`adminEnabled` clauses for scoped rollout, then start a `sim--fast` deployment (see the AppConfig runbook in the infra README — same flow as `access-control`). The fallback secret only applies when AppConfig is disabled. 5. **Test.** Add a case to `apps/sim/lib/core/config/feature-flags.test.ts` that matches the chosen granularity. For a global flag, exercise `isFeatureEnabled('')` with an AppConfig `enabled` rule and toggle the fallback secret for the off-AppConfig path. For scoped rollout, cover only the selected clauses and mock `isPlatformAdmin` when testing `adminEnabled`. @@ -90,6 +91,6 @@ Critically, **none of this is expressible in code** — gating (especially `admi - Flag keys are `kebab-case`. - Never read flags via raw `fetch` or a new AppConfig client — always go through `isFeatureEnabled` / `getFeatureFlags`. -- Never bake gating into code. The fallback is a single boolean secret; org/user/admin scoping is AppConfig-only. +- Never bake gating into code. The fallback is a single boolean secret; workspace/org/user/admin scoping is AppConfig-only. - Never add or propagate request context unless the user chose scoped rollout. - The admin check reads the DB **replica** (`dbReplica`) and is resolved lazily, so an admin-gated flag adds at most one cheap replica read, and only when `adminEnabled` is the deciding clause. diff --git a/apps/sim/lib/core/config/appconfig-rules.test.ts b/apps/sim/lib/core/config/appconfig-rules.test.ts index ab79359000d..398e93cfb40 100644 --- a/apps/sim/lib/core/config/appconfig-rules.test.ts +++ b/apps/sim/lib/core/config/appconfig-rules.test.ts @@ -24,6 +24,13 @@ describe('normalizeRule', () => { orgIds: ['Org_1', 'org_1', 'org_2'], }) }) + + it('normalizes the workspaceIds allowlist', () => { + expect(normalizeRule({ workspaceIds: [' ws_1 ', 'ws_1', ''] })).toEqual({ + workspaceIds: ['ws_1'], + }) + expect(normalizeRule({ workspaceIds: 'ws_1' })).toEqual({}) + }) }) describe('parseGateConfig', () => { @@ -61,6 +68,12 @@ describe('matchesRule', () => { expect(matchesRule({ orgIds: ['o1'] }, {}, false)).toBe(false) }) + it('matches the workspaceId allowlist', () => { + expect(matchesRule({ workspaceIds: ['w1'] }, { workspaceId: 'w1' }, false)).toBe(true) + expect(matchesRule({ workspaceIds: ['w1'] }, { workspaceId: 'w2' }, false)).toBe(false) + expect(matchesRule({ workspaceIds: ['w1'] }, {}, false)).toBe(false) + }) + it('matches the admin clause only with the supplied isAdmin', () => { expect(matchesRule({ adminEnabled: true }, { userId: 'u1' }, true)).toBe(true) expect(matchesRule({ adminEnabled: true }, { userId: 'u1' }, false)).toBe(false) diff --git a/apps/sim/lib/core/config/appconfig-rules.ts b/apps/sim/lib/core/config/appconfig-rules.ts index 34ceec5173a..786a5ec617f 100644 --- a/apps/sim/lib/core/config/appconfig-rules.ts +++ b/apps/sim/lib/core/config/appconfig-rules.ts @@ -10,11 +10,12 @@ /** * A single gating rule. A gate is open for a context when ANY clause matches: - * the global `enabled` default, the org/user allowlists, or `adminEnabled` for - * platform admins. An absent clause never matches. + * the global `enabled` default, the workspace/org/user allowlists, or + * `adminEnabled` for platform admins. An absent clause never matches. */ export interface AppConfigGateRule { enabled?: boolean + workspaceIds?: string[] orgIds?: string[] userIds?: string[] adminEnabled?: boolean @@ -28,6 +29,7 @@ export interface AppConfigGateRule { export interface AppConfigGateContext { userId?: string | null orgId?: string | null + workspaceId?: string | null isAdmin?: boolean } @@ -44,6 +46,8 @@ export function normalizeRule(value: unknown): AppConfigGateRule | null { const rule: AppConfigGateRule = {} if (typeof obj.enabled === 'boolean') rule.enabled = obj.enabled if (typeof obj.adminEnabled === 'boolean') rule.adminEnabled = obj.adminEnabled + const workspaceIds = normalizeIds(obj.workspaceIds) + if (workspaceIds) rule.workspaceIds = workspaceIds const orgIds = normalizeIds(obj.orgIds) if (orgIds) rule.orgIds = orgIds const userIds = normalizeIds(obj.userIds) @@ -75,6 +79,7 @@ export function matchesRule( if (rule.enabled) return true if (ctx.userId && rule.userIds?.includes(ctx.userId)) return true if (ctx.orgId && rule.orgIds?.includes(ctx.orgId)) return true + if (ctx.workspaceId && rule.workspaceIds?.includes(ctx.workspaceId)) return true if (rule.adminEnabled && isAdmin) return true return false } diff --git a/apps/sim/lib/core/config/feature-flags.test.ts b/apps/sim/lib/core/config/feature-flags.test.ts index b55b7b1ee0c..6023ac5812e 100644 --- a/apps/sim/lib/core/config/feature-flags.test.ts +++ b/apps/sim/lib/core/config/feature-flags.test.ts @@ -130,10 +130,24 @@ describe('isFeatureEnabled', () => { expect(await isFeatureEnabled('credential-groups')).toBe(true) }) - it('uses only the global AppConfig clause', async () => { + it('uses the global AppConfig clause', async () => { withAppConfig({ 'credential-groups': { enabled: true } }) expect(await isFeatureEnabled('credential-groups')).toBe(true) }) + + it('opens for an allowlisted workspace only', async () => { + withAppConfig({ 'credential-groups': { workspaceIds: ['ws-1'] } }) + expect(await isFeatureEnabled('credential-groups', { workspaceId: 'ws-1' })).toBe(true) + expect(await isFeatureEnabled('credential-groups', { workspaceId: 'ws-2' })).toBe(false) + expect(await isFeatureEnabled('credential-groups')).toBe(false) + }) + }) + + it('matches the workspaceIds clause', async () => { + withAppConfig({ f: { workspaceIds: ['ws-1'] } }) + expect(await enabled('f', { workspaceId: 'ws-1' })).toBe(true) + expect(await enabled('f', { workspaceId: 'ws-2' })).toBe(false) + expect(await enabled('f', { userId: 'ws-1' })).toBe(false) }) it('returns false for an unknown flag', async () => { diff --git a/apps/sim/lib/core/config/feature-flags.ts b/apps/sim/lib/core/config/feature-flags.ts index b9f75143ff0..b610508b7ee 100644 --- a/apps/sim/lib/core/config/feature-flags.ts +++ b/apps/sim/lib/core/config/feature-flags.ts @@ -13,8 +13,8 @@ const FEATURE_FLAGS_PROFILE = 'feature-flags' /** * A single flag's gating rule. A flag is ON for a context when ANY clause matches: - * the global `enabled` default, the org/user allowlists, or `adminEnabled` for - * platform admins. An absent clause never matches. Shape shared with the other + * the global `enabled` default, the workspace/org/user allowlists, or + * `adminEnabled` for platform admins. An absent clause never matches. Shape shared with the other * AppConfig gating documents via {@link AppConfigGateRule}. */ export type FeatureFlagRule = AppConfigGateRule @@ -33,7 +33,7 @@ export type FeatureFlagContext = AppConfigGateContext * AppConfig is not the source of truth (self-hosted/OSS, local dev, or hosted * without APPCONFIG_*). A truthy secret turns the flag on globally. * - * Gating by org/user/admin is available ONLY through the hosted AppConfig document + * Gating by workspace/org/user/admin is available ONLY through the hosted AppConfig document * — it deliberately cannot be expressed here, so no environment can grant (e.g.) * admin access from a code literal. To add a flag, register its name and the secret * to fall back on. @@ -44,7 +44,7 @@ export type FeatureFlagContext = AppConfigGateContext * `fallback` secret consulted when AppConfig isn't the source of truth (truthy ⇒ on * globally). * - * Gating by org/user/admin is deliberately NOT part of a definition — it lives only + * Gating by workspace/org/user/admin is deliberately NOT part of a definition — it lives only * in the hosted AppConfig document, so no environment can grant access from a code * literal. */ @@ -75,7 +75,8 @@ const FEATURE_FLAGS = { 'credential-groups': { description: 'Workspace-owned collections that gather managed OAuth credentials from external users. ' + - 'Global on/off only; hosted workspaces must also have an Enterprise subscription.', + 'Gated by workspaceId via AppConfig (or globally); hosted workspaces must also have an ' + + 'Enterprise subscription. Off-AppConfig falls back to CREDENTIAL_GROUPS.', fallback: 'CREDENTIAL_GROUPS', }, } satisfies Record @@ -108,8 +109,8 @@ async function resolveAdmin(userId: string): Promise { } /** - * The admin clause is resolved last and lazily: a global/userId/orgId match - * short-circuits before any DB read, a rule without `adminEnabled` never queries, + * The admin clause is resolved last and lazily: a global/userId/orgId/workspaceId + * match short-circuits before any DB read, a rule without `adminEnabled` never queries, * and a missing `userId` resolves to `false` without a query. */ async function evaluate( diff --git a/apps/sim/lib/credential-groups/application/context.ts b/apps/sim/lib/credential-groups/application/context.ts index 3329e7a3b9d..7ca35722a39 100644 --- a/apps/sim/lib/credential-groups/application/context.ts +++ b/apps/sim/lib/credential-groups/application/context.ts @@ -10,7 +10,7 @@ import { loadActiveWorkspaceApplicationContext } from '@/lib/workspaces/applicat export async function requireCredentialGroupsAvailable(workspaceId: string): Promise { const ownerBilling = await getWorkspaceOwnerSubscriptionAccess(workspaceId) - const availability = await resolveCredentialGroupsAvailability(ownerBilling) + const availability = await resolveCredentialGroupsAvailability({ workspaceId, ownerBilling }) if (!availability.available) { const message = availability.reason === 'enterprise_plan_required' @@ -22,7 +22,7 @@ export async function requireCredentialGroupsAvailable(workspaceId: string): Pro export async function requireCredentialGroupSettingsAvailable(workspaceId: string): Promise { const ownerBilling = await getWorkspaceOwnerSubscriptionAccess(workspaceId) - if (!(await isCredentialGroupsAvailable(ownerBilling))) { + if (!(await isCredentialGroupsAvailable({ workspaceId, ownerBilling }))) { throw new OrchestrationError('not_found', 'Credential Groups are not available') } } diff --git a/apps/sim/lib/credential-groups/application/slack-managed-users.ts b/apps/sim/lib/credential-groups/application/slack-managed-users.ts index a448edbd284..8480195f87d 100644 --- a/apps/sim/lib/credential-groups/application/slack-managed-users.ts +++ b/apps/sim/lib/credential-groups/application/slack-managed-users.ts @@ -18,7 +18,7 @@ import { loadActiveWorkspaceApplicationContext } from '@/lib/workspaces/applicat async function requireCredentialGroups(workspaceId: string): Promise { const ownerBilling = await getWorkspaceOwnerSubscriptionAccess(workspaceId) - if (!(await isCredentialGroupsAvailable(ownerBilling))) { + if (!(await isCredentialGroupsAvailable({ workspaceId, ownerBilling }))) { throw new OrchestrationError('not_found', 'Credential Groups are not available') } } diff --git a/apps/sim/lib/credential-groups/availability.test.ts b/apps/sim/lib/credential-groups/availability.test.ts index 51960c8184d..2a51f7c6225 100644 --- a/apps/sim/lib/credential-groups/availability.test.ts +++ b/apps/sim/lib/credential-groups/availability.test.ts @@ -25,7 +25,12 @@ describe('resolveCredentialGroupsAvailability', () => { it('attributes a disabled feature flag before considering the plan', async () => { mockIsFeatureEnabled.mockResolvedValue(false) - await expect(resolveCredentialGroupsAvailability({ isEnterprise: false })).resolves.toEqual({ + await expect( + resolveCredentialGroupsAvailability({ + workspaceId: 'ws-1', + ownerBilling: { isEnterprise: false }, + }) + ).resolves.toEqual({ available: false, reason: 'feature_disabled', }) @@ -34,16 +39,37 @@ describe('resolveCredentialGroupsAvailability', () => { it('requires Enterprise when the hosted feature is enabled', async () => { mockIsFeatureEnabled.mockResolvedValue(true) - await expect(resolveCredentialGroupsAvailability({ isEnterprise: false })).resolves.toEqual({ + await expect( + resolveCredentialGroupsAvailability({ + workspaceId: 'ws-1', + ownerBilling: { isEnterprise: false }, + }) + ).resolves.toEqual({ available: false, reason: 'enterprise_plan_required', }) }) + it('evaluates the flag against the workspace id', async () => { + mockIsFeatureEnabled.mockResolvedValue(true) + + await resolveCredentialGroupsAvailability({ + workspaceId: 'ws-1', + ownerBilling: { isEnterprise: true }, + }) + + expect(mockIsFeatureEnabled).toHaveBeenCalledWith('credential-groups', { workspaceId: 'ws-1' }) + }) + it('allows Enterprise workspaces when the hosted feature is enabled', async () => { mockIsFeatureEnabled.mockResolvedValue(true) - await expect(resolveCredentialGroupsAvailability({ isEnterprise: true })).resolves.toEqual({ + await expect( + resolveCredentialGroupsAvailability({ + workspaceId: 'ws-1', + ownerBilling: { isEnterprise: true }, + }) + ).resolves.toEqual({ available: true, }) }) diff --git a/apps/sim/lib/credential-groups/availability.ts b/apps/sim/lib/credential-groups/availability.ts index cc56832f87c..76852800476 100644 --- a/apps/sim/lib/credential-groups/availability.ts +++ b/apps/sim/lib/credential-groups/availability.ts @@ -5,10 +5,21 @@ export type CredentialGroupsAvailability = | { available: true } | { available: false; reason: 'feature_disabled' | 'enterprise_plan_required' } -export async function resolveCredentialGroupsAvailability(ownerBilling: { - isEnterprise: boolean -}): Promise { - if (!(await isFeatureEnabled('credential-groups'))) { +/** + * The workspace the gate is evaluated for. `workspaceId` is required so no call + * site can silently fall back to the global clause and reveal the feature to a + * workspace the AppConfig `credential-groups` allowlist does not name. + */ +export interface CredentialGroupsAvailabilityInput { + workspaceId: string + ownerBilling: { isEnterprise: boolean } +} + +export async function resolveCredentialGroupsAvailability({ + workspaceId, + ownerBilling, +}: CredentialGroupsAvailabilityInput): Promise { + if (!(await isFeatureEnabled('credential-groups', { workspaceId }))) { return { available: false, reason: 'feature_disabled' } } if (isHosted && !ownerBilling.isEnterprise) { @@ -17,9 +28,12 @@ export async function resolveCredentialGroupsAvailability(ownerBilling: { return { available: true } } -/** Credential Groups are globally gated and restricted to Enterprise workspaces on Sim Cloud. */ -export async function isCredentialGroupsAvailable(ownerBilling: { - isEnterprise: boolean -}): Promise { - return (await resolveCredentialGroupsAvailability(ownerBilling)).available +/** + * Credential Groups are gated per workspace (globally or by the AppConfig + * `workspaceIds` allowlist) and restricted to Enterprise workspaces on Sim Cloud. + */ +export async function isCredentialGroupsAvailable( + input: CredentialGroupsAvailabilityInput +): Promise { + return (await resolveCredentialGroupsAvailability(input)).available } diff --git a/apps/sim/lib/credential-groups/enrollments.ts b/apps/sim/lib/credential-groups/enrollments.ts index c389d884b8b..093ddaad70a 100644 --- a/apps/sim/lib/credential-groups/enrollments.ts +++ b/apps/sim/lib/credential-groups/enrollments.ts @@ -227,7 +227,8 @@ async function resolvePublicEnrollmentRowByIdentity( if (row.enrollment.invitationExpiresAt.getTime() <= Date.now()) return null const ownerBilling = await getWorkspaceOwnerSubscriptionAccess(row.workspaceId) - if (!(await isCredentialGroupsAvailable(ownerBilling))) return null + if (!(await isCredentialGroupsAvailable({ workspaceId: row.workspaceId, ownerBilling }))) + return null return row } diff --git a/apps/sim/lib/credentials/managed-oauth.ts b/apps/sim/lib/credentials/managed-oauth.ts index 4edca04150e..134b1f711e6 100644 --- a/apps/sim/lib/credentials/managed-oauth.ts +++ b/apps/sim/lib/credentials/managed-oauth.ts @@ -290,7 +290,7 @@ export async function resolveManagedOAuthToken( } const ownerBilling = await getWorkspaceOwnerSubscriptionAccess(initial.workspaceId) - if (!(await isCredentialGroupsAvailable(ownerBilling))) { + if (!(await isCredentialGroupsAvailable({ workspaceId: initial.workspaceId, ownerBilling }))) { throw new ManagedOAuthCredentialError( 'MANAGED_CREDENTIAL_UNAVAILABLE', 'Managed credentials are not available for this workspace', diff --git a/apps/sim/lib/workspaces/host-context.ts b/apps/sim/lib/workspaces/host-context.ts index a9306e22dc7..0c66496eb27 100644 --- a/apps/sim/lib/workspaces/host-context.ts +++ b/apps/sim/lib/workspaces/host-context.ts @@ -28,7 +28,7 @@ async function resolveWorkspaceHostContextForViewer( ? getOrganizationSettingsAccess(hostOrganizationId, userId) : Promise.resolve({ role: null, isMember: false, isAdmin: false }), ]) - const credentialGroupsAvailable = await isCredentialGroupsAvailable(ownerBilling) + const credentialGroupsAvailable = await isCredentialGroupsAvailable({ workspaceId, ownerBilling }) return { workspace: {