Skip to content

Commit ba0068c

Browse files
improvement(config): gate feature flags by workspace id
Add a workspaceIds allowlist clause to the shared AppConfig gate rules and apply it to the credential-groups flag, so the feature can be enabled for a specific workspace without a global rollout.
1 parent fa00b63 commit ba0068c

12 files changed

Lines changed: 118 additions & 43 deletions

File tree

.agents/skills/add-feature-flag/SKILL.md

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
11
---
22
name: add-feature-flag
3-
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
3+
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
44
argument-hint: <flag-name>
55
---
66

77
# Add Feature Flag Skill
88

9-
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).
9+
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).
1010

1111
## When to use this vs `env-flags.ts`
1212

13-
- **Feature flag** (`@/lib/core/config/feature-flags.ts`): runtime global on/off by default, optionally scoped by `userId`/`orgId`/admin. This skill.
13+
- **Feature flag** (`@/lib/core/config/feature-flags.ts`): runtime global on/off by default, optionally scoped by `workspaceId`/`userId`/`orgId`/admin. This skill.
1414
- **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.**
1515

1616
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
2121

2222
```ts
2323
interface FeatureFlagRule {
24-
enabled?: boolean // global default for everyone
25-
orgIds?: string[] // allowlisted organization ids
26-
userIds?: string[] // allowlisted user ids
27-
adminEnabled?: boolean // platform admins (user.role === 'admin')
24+
enabled?: boolean // global default for everyone
25+
workspaceIds?: string[] // allowlisted workspace ids
26+
orgIds?: string[] // allowlisted organization ids
27+
userIds?: string[] // allowlisted user ids
28+
adminEnabled?: boolean // platform admins (user.role === 'admin')
2829
}
2930
```
3031

@@ -34,10 +35,10 @@ Critically, **none of this is expressible in code** — gating (especially `admi
3435

3536
1. **Confirm the granularity before editing code.** If the user has not already specified it, stop and ask:
3637

37-
> Should `<flag-name>` be a global on/off flag (recommended), or does it need rollout targeting by organization, user, and/or platform admin?
38+
> Should `<flag-name>` be a global on/off flag (recommended), or does it need rollout targeting by workspace, organization, user, and/or platform admin?
3839
39-
- Recommend **global**. Do not infer scoped gating merely because the call site already has a user or organization id.
40-
- 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.
40+
- Recommend **global**. Do not infer scoped gating merely because the call site already has a workspace, user, or organization id.
41+
- 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.
4142
- If the user wants a fixed per-deployment toggle rather than a runtime AppConfig flag, use `env-flags.ts` instead.
4243

4344
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
5152
}
5253
```
5354

54-
`fallback` is the env/secret key (typed as `keyof typeof env`), so add `<FLAG_SECRET>` 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 `<flag-name>` a valid `FeatureFlagName`.
55+
`fallback` is the env/secret key (typed as `keyof typeof env`), so add `<FLAG_SECRET>` 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 `<flag-name>` a valid `FeatureFlagName`.
5556

5657
3. **Gate the call site at the chosen granularity.** For the recommended global mode, pass no context:
5758

@@ -70,17 +71,17 @@ Critically, **none of this is expressible in code** — gating (especially `admi
7071
```ts
7172
import { isFeatureEnabled } from '@/lib/core/config/feature-flags'
7273

73-
if (await isFeatureEnabled('<flag-name>', { userId, orgId })) {
74+
if (await isFeatureEnabled('<flag-name>', { workspaceId, userId, orgId })) {
7475
// gated behavior
7576
}
7677
```
7778

78-
- Organization targeting uses `orgId`; user and platform-admin targeting require `userId`.
79+
- Workspace targeting uses `workspaceId`; organization targeting uses `orgId`; user and platform-admin targeting require `userId`.
7980
- 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.
8081
- Admin routes that already know the caller is an admin may pass `{ userId, isAdmin: true }` to skip the role lookup.
8182
- **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.
8283

83-
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-<env>-fast` deployment (see the AppConfig runbook in the infra README — same flow as `access-control`). The fallback secret only applies when AppConfig is disabled.
84+
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-<env>-fast` deployment (see the AppConfig runbook in the infra README — same flow as `access-control`). The fallback secret only applies when AppConfig is disabled.
8485

8586
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('<flag-name>')` 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`.
8687

@@ -90,6 +91,6 @@ Critically, **none of this is expressible in code** — gating (especially `admi
9091

9192
- Flag keys are `kebab-case`.
9293
- Never read flags via raw `fetch` or a new AppConfig client — always go through `isFeatureEnabled` / `getFeatureFlags`.
93-
- Never bake gating into code. The fallback is a single boolean secret; org/user/admin scoping is AppConfig-only.
94+
- Never bake gating into code. The fallback is a single boolean secret; workspace/org/user/admin scoping is AppConfig-only.
9495
- Never add or propagate request context unless the user chose scoped rollout.
9596
- 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.

apps/sim/lib/core/config/appconfig-rules.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@ describe('normalizeRule', () => {
2424
orgIds: ['Org_1', 'org_1', 'org_2'],
2525
})
2626
})
27+
28+
it('normalizes the workspaceIds allowlist', () => {
29+
expect(normalizeRule({ workspaceIds: [' ws_1 ', 'ws_1', ''] })).toEqual({
30+
workspaceIds: ['ws_1'],
31+
})
32+
expect(normalizeRule({ workspaceIds: 'ws_1' })).toEqual({})
33+
})
2734
})
2835

2936
describe('parseGateConfig', () => {
@@ -61,6 +68,12 @@ describe('matchesRule', () => {
6168
expect(matchesRule({ orgIds: ['o1'] }, {}, false)).toBe(false)
6269
})
6370

71+
it('matches the workspaceId allowlist', () => {
72+
expect(matchesRule({ workspaceIds: ['w1'] }, { workspaceId: 'w1' }, false)).toBe(true)
73+
expect(matchesRule({ workspaceIds: ['w1'] }, { workspaceId: 'w2' }, false)).toBe(false)
74+
expect(matchesRule({ workspaceIds: ['w1'] }, {}, false)).toBe(false)
75+
})
76+
6477
it('matches the admin clause only with the supplied isAdmin', () => {
6578
expect(matchesRule({ adminEnabled: true }, { userId: 'u1' }, true)).toBe(true)
6679
expect(matchesRule({ adminEnabled: true }, { userId: 'u1' }, false)).toBe(false)

apps/sim/lib/core/config/appconfig-rules.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,12 @@
1010

1111
/**
1212
* A single gating rule. A gate is open for a context when ANY clause matches:
13-
* the global `enabled` default, the org/user allowlists, or `adminEnabled` for
14-
* platform admins. An absent clause never matches.
13+
* the global `enabled` default, the workspace/org/user allowlists, or
14+
* `adminEnabled` for platform admins. An absent clause never matches.
1515
*/
1616
export interface AppConfigGateRule {
1717
enabled?: boolean
18+
workspaceIds?: string[]
1819
orgIds?: string[]
1920
userIds?: string[]
2021
adminEnabled?: boolean
@@ -28,6 +29,7 @@ export interface AppConfigGateRule {
2829
export interface AppConfigGateContext {
2930
userId?: string | null
3031
orgId?: string | null
32+
workspaceId?: string | null
3133
isAdmin?: boolean
3234
}
3335

@@ -44,6 +46,8 @@ export function normalizeRule(value: unknown): AppConfigGateRule | null {
4446
const rule: AppConfigGateRule = {}
4547
if (typeof obj.enabled === 'boolean') rule.enabled = obj.enabled
4648
if (typeof obj.adminEnabled === 'boolean') rule.adminEnabled = obj.adminEnabled
49+
const workspaceIds = normalizeIds(obj.workspaceIds)
50+
if (workspaceIds) rule.workspaceIds = workspaceIds
4751
const orgIds = normalizeIds(obj.orgIds)
4852
if (orgIds) rule.orgIds = orgIds
4953
const userIds = normalizeIds(obj.userIds)
@@ -75,6 +79,7 @@ export function matchesRule(
7579
if (rule.enabled) return true
7680
if (ctx.userId && rule.userIds?.includes(ctx.userId)) return true
7781
if (ctx.orgId && rule.orgIds?.includes(ctx.orgId)) return true
82+
if (ctx.workspaceId && rule.workspaceIds?.includes(ctx.workspaceId)) return true
7883
if (rule.adminEnabled && isAdmin) return true
7984
return false
8085
}

apps/sim/lib/core/config/feature-flags.test.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,10 +130,24 @@ describe('isFeatureEnabled', () => {
130130
expect(await isFeatureEnabled('credential-groups')).toBe(true)
131131
})
132132

133-
it('uses only the global AppConfig clause', async () => {
133+
it('uses the global AppConfig clause', async () => {
134134
withAppConfig({ 'credential-groups': { enabled: true } })
135135
expect(await isFeatureEnabled('credential-groups')).toBe(true)
136136
})
137+
138+
it('opens for an allowlisted workspace only', async () => {
139+
withAppConfig({ 'credential-groups': { workspaceIds: ['ws-1'] } })
140+
expect(await isFeatureEnabled('credential-groups', { workspaceId: 'ws-1' })).toBe(true)
141+
expect(await isFeatureEnabled('credential-groups', { workspaceId: 'ws-2' })).toBe(false)
142+
expect(await isFeatureEnabled('credential-groups')).toBe(false)
143+
})
144+
})
145+
146+
it('matches the workspaceIds clause', async () => {
147+
withAppConfig({ f: { workspaceIds: ['ws-1'] } })
148+
expect(await enabled('f', { workspaceId: 'ws-1' })).toBe(true)
149+
expect(await enabled('f', { workspaceId: 'ws-2' })).toBe(false)
150+
expect(await enabled('f', { userId: 'ws-1' })).toBe(false)
137151
})
138152

139153
it('returns false for an unknown flag', async () => {

apps/sim/lib/core/config/feature-flags.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ const FEATURE_FLAGS_PROFILE = 'feature-flags'
1313

1414
/**
1515
* A single flag's gating rule. A flag is ON for a context when ANY clause matches:
16-
* the global `enabled` default, the org/user allowlists, or `adminEnabled` for
17-
* platform admins. An absent clause never matches. Shape shared with the other
16+
* the global `enabled` default, the workspace/org/user allowlists, or
17+
* `adminEnabled` for platform admins. An absent clause never matches. Shape shared with the other
1818
* AppConfig gating documents via {@link AppConfigGateRule}.
1919
*/
2020
export type FeatureFlagRule = AppConfigGateRule
@@ -33,7 +33,7 @@ export type FeatureFlagContext = AppConfigGateContext
3333
* AppConfig is not the source of truth (self-hosted/OSS, local dev, or hosted
3434
* without APPCONFIG_*). A truthy secret turns the flag on globally.
3535
*
36-
* Gating by org/user/admin is available ONLY through the hosted AppConfig document
36+
* Gating by workspace/org/user/admin is available ONLY through the hosted AppConfig document
3737
* — it deliberately cannot be expressed here, so no environment can grant (e.g.)
3838
* admin access from a code literal. To add a flag, register its name and the secret
3939
* to fall back on.
@@ -44,7 +44,7 @@ export type FeatureFlagContext = AppConfigGateContext
4444
* `fallback` secret consulted when AppConfig isn't the source of truth (truthy ⇒ on
4545
* globally).
4646
*
47-
* Gating by org/user/admin is deliberately NOT part of a definition — it lives only
47+
* Gating by workspace/org/user/admin is deliberately NOT part of a definition — it lives only
4848
* in the hosted AppConfig document, so no environment can grant access from a code
4949
* literal.
5050
*/
@@ -75,7 +75,8 @@ const FEATURE_FLAGS = {
7575
'credential-groups': {
7676
description:
7777
'Workspace-owned collections that gather managed OAuth credentials from external users. ' +
78-
'Global on/off only; hosted workspaces must also have an Enterprise subscription.',
78+
'Gated by workspaceId via AppConfig (or globally); hosted workspaces must also have an ' +
79+
'Enterprise subscription. Off-AppConfig falls back to CREDENTIAL_GROUPS.',
7980
fallback: 'CREDENTIAL_GROUPS',
8081
},
8182
} satisfies Record<string, FeatureFlagDefinition>
@@ -108,8 +109,8 @@ async function resolveAdmin(userId: string): Promise<boolean> {
108109
}
109110

110111
/**
111-
* The admin clause is resolved last and lazily: a global/userId/orgId match
112-
* short-circuits before any DB read, a rule without `adminEnabled` never queries,
112+
* The admin clause is resolved last and lazily: a global/userId/orgId/workspaceId
113+
* match short-circuits before any DB read, a rule without `adminEnabled` never queries,
113114
* and a missing `userId` resolves to `false` without a query.
114115
*/
115116
async function evaluate(

apps/sim/lib/credential-groups/application/context.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { loadActiveWorkspaceApplicationContext } from '@/lib/workspaces/applicat
1010

1111
export async function requireCredentialGroupsAvailable(workspaceId: string): Promise<void> {
1212
const ownerBilling = await getWorkspaceOwnerSubscriptionAccess(workspaceId)
13-
const availability = await resolveCredentialGroupsAvailability(ownerBilling)
13+
const availability = await resolveCredentialGroupsAvailability({ workspaceId, ownerBilling })
1414
if (!availability.available) {
1515
const message =
1616
availability.reason === 'enterprise_plan_required'
@@ -22,7 +22,7 @@ export async function requireCredentialGroupsAvailable(workspaceId: string): Pro
2222

2323
export async function requireCredentialGroupSettingsAvailable(workspaceId: string): Promise<void> {
2424
const ownerBilling = await getWorkspaceOwnerSubscriptionAccess(workspaceId)
25-
if (!(await isCredentialGroupsAvailable(ownerBilling))) {
25+
if (!(await isCredentialGroupsAvailable({ workspaceId, ownerBilling }))) {
2626
throw new OrchestrationError('not_found', 'Credential Groups are not available')
2727
}
2828
}

apps/sim/lib/credential-groups/application/slack-managed-users.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { loadActiveWorkspaceApplicationContext } from '@/lib/workspaces/applicat
1818

1919
async function requireCredentialGroups(workspaceId: string): Promise<void> {
2020
const ownerBilling = await getWorkspaceOwnerSubscriptionAccess(workspaceId)
21-
if (!(await isCredentialGroupsAvailable(ownerBilling))) {
21+
if (!(await isCredentialGroupsAvailable({ workspaceId, ownerBilling }))) {
2222
throw new OrchestrationError('not_found', 'Credential Groups are not available')
2323
}
2424
}

apps/sim/lib/credential-groups/availability.test.ts

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,12 @@ describe('resolveCredentialGroupsAvailability', () => {
2525
it('attributes a disabled feature flag before considering the plan', async () => {
2626
mockIsFeatureEnabled.mockResolvedValue(false)
2727

28-
await expect(resolveCredentialGroupsAvailability({ isEnterprise: false })).resolves.toEqual({
28+
await expect(
29+
resolveCredentialGroupsAvailability({
30+
workspaceId: 'ws-1',
31+
ownerBilling: { isEnterprise: false },
32+
})
33+
).resolves.toEqual({
2934
available: false,
3035
reason: 'feature_disabled',
3136
})
@@ -34,16 +39,37 @@ describe('resolveCredentialGroupsAvailability', () => {
3439
it('requires Enterprise when the hosted feature is enabled', async () => {
3540
mockIsFeatureEnabled.mockResolvedValue(true)
3641

37-
await expect(resolveCredentialGroupsAvailability({ isEnterprise: false })).resolves.toEqual({
42+
await expect(
43+
resolveCredentialGroupsAvailability({
44+
workspaceId: 'ws-1',
45+
ownerBilling: { isEnterprise: false },
46+
})
47+
).resolves.toEqual({
3848
available: false,
3949
reason: 'enterprise_plan_required',
4050
})
4151
})
4252

53+
it('evaluates the flag against the workspace id', async () => {
54+
mockIsFeatureEnabled.mockResolvedValue(true)
55+
56+
await resolveCredentialGroupsAvailability({
57+
workspaceId: 'ws-1',
58+
ownerBilling: { isEnterprise: true },
59+
})
60+
61+
expect(mockIsFeatureEnabled).toHaveBeenCalledWith('credential-groups', { workspaceId: 'ws-1' })
62+
})
63+
4364
it('allows Enterprise workspaces when the hosted feature is enabled', async () => {
4465
mockIsFeatureEnabled.mockResolvedValue(true)
4566

46-
await expect(resolveCredentialGroupsAvailability({ isEnterprise: true })).resolves.toEqual({
67+
await expect(
68+
resolveCredentialGroupsAvailability({
69+
workspaceId: 'ws-1',
70+
ownerBilling: { isEnterprise: true },
71+
})
72+
).resolves.toEqual({
4773
available: true,
4874
})
4975
})

apps/sim/lib/credential-groups/availability.ts

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,21 @@ export type CredentialGroupsAvailability =
55
| { available: true }
66
| { available: false; reason: 'feature_disabled' | 'enterprise_plan_required' }
77

8-
export async function resolveCredentialGroupsAvailability(ownerBilling: {
9-
isEnterprise: boolean
10-
}): Promise<CredentialGroupsAvailability> {
11-
if (!(await isFeatureEnabled('credential-groups'))) {
8+
/**
9+
* The workspace the gate is evaluated for. `workspaceId` is required so no call
10+
* site can silently fall back to the global clause and reveal the feature to a
11+
* workspace the AppConfig `credential-groups` allowlist does not name.
12+
*/
13+
export interface CredentialGroupsAvailabilityInput {
14+
workspaceId: string
15+
ownerBilling: { isEnterprise: boolean }
16+
}
17+
18+
export async function resolveCredentialGroupsAvailability({
19+
workspaceId,
20+
ownerBilling,
21+
}: CredentialGroupsAvailabilityInput): Promise<CredentialGroupsAvailability> {
22+
if (!(await isFeatureEnabled('credential-groups', { workspaceId }))) {
1223
return { available: false, reason: 'feature_disabled' }
1324
}
1425
if (isHosted && !ownerBilling.isEnterprise) {
@@ -17,9 +28,12 @@ export async function resolveCredentialGroupsAvailability(ownerBilling: {
1728
return { available: true }
1829
}
1930

20-
/** Credential Groups are globally gated and restricted to Enterprise workspaces on Sim Cloud. */
21-
export async function isCredentialGroupsAvailable(ownerBilling: {
22-
isEnterprise: boolean
23-
}): Promise<boolean> {
24-
return (await resolveCredentialGroupsAvailability(ownerBilling)).available
31+
/**
32+
* Credential Groups are gated per workspace (globally or by the AppConfig
33+
* `workspaceIds` allowlist) and restricted to Enterprise workspaces on Sim Cloud.
34+
*/
35+
export async function isCredentialGroupsAvailable(
36+
input: CredentialGroupsAvailabilityInput
37+
): Promise<boolean> {
38+
return (await resolveCredentialGroupsAvailability(input)).available
2539
}

0 commit comments

Comments
 (0)