Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions .claude/commands/add-feature-flag.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
---
description: Add a runtime gated feature flag (AppConfig-backed on prod, secret fallback off-prod), gated by org id, user id, or admin
argument-hint: <flag-name>
---

# Add Feature Flag Skill

You add a **runtime, gated feature flag** to Sim — one that can be turned on for specific orgs, users, or admins and changed on prod with no redeploy (AWS AppConfig). 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`): per-request, gated by `userId`/`orgId`/admin, changeable at runtime. 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.

## The flag model

A flag's **gating rule lives only in the hosted AppConfig document**. It is ON for a context when any clause matches:

```ts
interface FeatureFlagRule {
enabled?: boolean // global default for everyone
orgIds?: string[] // allowlisted organization ids
userIds?: string[] // allowlisted user ids
admins?: boolean // platform admins (user.role === 'admin')
}
```

Critically, **none of this is expressible in code** — gating (especially `admins`) can only be set through AppConfig, so no environment can grant access from a code literal. Off-AppConfig (self-hosted/OSS/local), a flag is simply on or off, derived from its fallback secret.

## Steps

1. **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):

```ts
const FEATURE_FLAGS = {
'<flag-name>': {
description: '<what this gates>',
fallback: '<FLAG_SECRET>',
},
}
```

`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`.

2. **Gate the call site.** Call `isFeatureEnabled` with whatever ids you have — admin status is resolved internally, so callers never pass it:

```ts
import { isFeatureEnabled } from '@/lib/core/config/feature-flags'

if (await isFeatureEnabled('<flag-name>', { userId, orgId })) {
// gated behavior
}
```

- 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.

3. **(Prod) configure in AppConfig.** The infra `feature-flags` profile schema is permissive, so a new flag needs **no infra change**. Operators add the flag under `flags` in the hosted `feature-flags` document — including any `orgIds`/`userIds`/`admins` gating — and 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.

4. **Test.** Add a case to `apps/sim/lib/core/config/feature-flags.test.ts`: use `withAppConfig({ flags: { ... } })` to cover the gating rule (mock `isPlatformAdmin` for the `admins` clause), and toggle the fallback secret to cover the off-AppConfig path.

5. **Clean up after rollout.** When the feature ships to everyone, delete the flag's entry from `FEATURE_FLAGS`, the `<FLAG_SECRET>` env entry, the AppConfig document, the call sites, and the test. Leaving dead flags around is the main failure mode of flag systems.

## Notes

- 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.
- 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 `admins` is the deciding clause.
67 changes: 67 additions & 0 deletions .cursor/commands/add-feature-flag.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# Add Feature Flag Skill

You add a **runtime, gated feature flag** to Sim — one that can be turned on for specific orgs, users, or admins and changed on prod with no redeploy (AWS AppConfig). 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`): per-request, gated by `userId`/`orgId`/admin, changeable at runtime. 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.

## The flag model

A flag's **gating rule lives only in the hosted AppConfig document**. It is ON for a context when any clause matches:

```ts
interface FeatureFlagRule {
enabled?: boolean // global default for everyone
orgIds?: string[] // allowlisted organization ids
userIds?: string[] // allowlisted user ids
admins?: boolean // platform admins (user.role === 'admin')
}
```

Critically, **none of this is expressible in code** — gating (especially `admins`) can only be set through AppConfig, so no environment can grant access from a code literal. Off-AppConfig (self-hosted/OSS/local), a flag is simply on or off, derived from its fallback secret.

## Steps

1. **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):

```ts
const FEATURE_FLAGS = {
'<flag-name>': {
description: '<what this gates>',
fallback: '<FLAG_SECRET>',
},
}
```

`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`.

2. **Gate the call site.** Call `isFeatureEnabled` with whatever ids you have — admin status is resolved internally, so callers never pass it:

```ts
import { isFeatureEnabled } from '@/lib/core/config/feature-flags'

if (await isFeatureEnabled('<flag-name>', { userId, orgId })) {
// gated behavior
}
```

- 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.

3. **(Prod) configure in AppConfig.** The infra `feature-flags` profile schema is permissive, so a new flag needs **no infra change**. Operators add the flag under `flags` in the hosted `feature-flags` document — including any `orgIds`/`userIds`/`admins` gating — and 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.

4. **Test.** Add a case to `apps/sim/lib/core/config/feature-flags.test.ts`: use `withAppConfig({ flags: { ... } })` to cover the gating rule (mock `isPlatformAdmin` for the `admins` clause), and toggle the fallback secret to cover the off-AppConfig path.

5. **Clean up after rollout.** When the feature ships to everyone, delete the flag's entry from `FEATURE_FLAGS`, the `<FLAG_SECRET>` env entry, the AppConfig document, the call sites, and the test. Leaving dead flags around is the main failure mode of flag systems.

## Notes

- 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.
- 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 `admins` is the deciding clause.
2 changes: 1 addition & 1 deletion .cursor/rules/sim-testing.mdc
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ vi.useFakeTimers()
| `@/lib/auth/hybrid` | `hybridAuthMock`, `hybridAuthMockFns` | `vi.mock('@/lib/auth/hybrid', () => hybridAuthMock)` |
| `@/lib/copilot/request/http` | `copilotHttpMock`, `copilotHttpMockFns` | `vi.mock('@/lib/copilot/request/http', () => copilotHttpMock)` |
| `@/lib/core/config/env` | `envMock`, `createEnvMock(overrides)` | `vi.mock('@/lib/core/config/env', () => envMock)` |
| `@/lib/core/config/feature-flags` | `featureFlagsMock` | `vi.mock('@/lib/core/config/feature-flags', () => featureFlagsMock)` |
| `@/lib/core/config/env-flags` | `featureFlagsMock` | `vi.mock('@/lib/core/config/env-flags', () => featureFlagsMock)` |
| `@/lib/core/config/redis` | `redisConfigMock`, `redisConfigMockFns` | `vi.mock('@/lib/core/config/redis', () => redisConfigMock)` |
| `@/lib/core/security/encryption` | `encryptionMock`, `encryptionMockFns` | `vi.mock('@/lib/core/security/encryption', () => encryptionMock)` |
| `@/lib/core/security/input-validation.server` | `inputValidationMock`, `inputValidationMockFns` | `vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock)` |
Expand Down
14 changes: 7 additions & 7 deletions .github/workflows/test-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ jobs:
- name: Install dependencies
run: bun install --frozen-lockfile

- name: Validate feature flags
- name: Validate env flags
run: |
FILE="apps/sim/lib/core/config/feature-flags.ts"
FILE="apps/sim/lib/core/config/env-flags.ts"
ERRORS=""

echo "Checking for hardcoded boolean feature flags..."
echo "Checking for hardcoded boolean env flags..."

# Use perl for multiline matching to catch both:
# export const isHosted = true
Expand All @@ -69,17 +69,17 @@ jobs:
HARDCODED=$(perl -0777 -ne 'while (/export const (is[A-Za-z]+)\s*=\s*\n?\s*(true|false)\b/g) { print " $1 = $2\n" }' "$FILE")

if [ -n "$HARDCODED" ]; then
ERRORS="${ERRORS}\n❌ Feature flags must not be hardcoded to boolean literals!\n\nFound hardcoded flags:\n${HARDCODED}\n\nFeature flags should derive their values from environment variables.\n"
ERRORS="${ERRORS}\n❌ Env flags must not be hardcoded to boolean literals!\n\nFound hardcoded flags:\n${HARDCODED}\n\nEnv flags should derive their values from environment variables.\n"
fi

echo "Checking feature flag naming conventions..."
echo "Checking env flag naming conventions..."

# Check that all export const (except functions) start with 'is'
# This finds exports like "export const someFlag" that don't start with "is" or "get"
BAD_NAMES=$(grep -E "^export const [a-z]" "$FILE" | grep -vE "^export const (is|get)" | sed 's/export const \([a-zA-Z]*\).*/ \1/')

if [ -n "$BAD_NAMES" ]; then
ERRORS="${ERRORS}\n❌ Feature flags must use 'is' prefix for boolean flags!\n\nFound incorrectly named flags:\n${BAD_NAMES}\n\nExample: 'hostedMode' should be 'isHostedMode'\n"
ERRORS="${ERRORS}\n❌ Env flags must use 'is' prefix for boolean flags!\n\nFound incorrectly named flags:\n${BAD_NAMES}\n\nExample: 'hostedMode' should be 'isHostedMode'\n"
fi

if [ -n "$ERRORS" ]; then
Expand All @@ -88,7 +88,7 @@ jobs:
exit 1
fi

echo "✅ All feature flags are properly configured"
echo "✅ All env flags are properly configured"

- name: Check block registry invariants
run: |
Expand Down
2 changes: 1 addition & 1 deletion apps/sim/app/(auth)/components/oauth-provider-checker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
isGoogleAuthDisabled,
isMicrosoftAuthDisabled,
isProd,
} from '@/lib/core/config/feature-flags'
} from '@/lib/core/config/env-flags'

export async function getOAuthProviderStatus() {
const githubAvailable =
Expand Down
2 changes: 1 addition & 1 deletion apps/sim/app/(auth)/signup/page.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Metadata } from 'next'
import { isEmailSignupDisabled, isRegistrationDisabled } from '@/lib/core/config/feature-flags'
import { isEmailSignupDisabled, isRegistrationDisabled } from '@/lib/core/config/env-flags'
import { getOAuthProviderStatus } from '@/app/(auth)/components/oauth-provider-checker'
import SignupForm from '@/app/(auth)/signup/signup-form'

Expand Down
2 changes: 1 addition & 1 deletion apps/sim/app/(auth)/verify/page.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Metadata } from 'next'
import { isEmailVerificationEnabled, isProd } from '@/lib/core/config/feature-flags'
import { isEmailVerificationEnabled, isProd } from '@/lib/core/config/env-flags'
import { hasEmailService } from '@/lib/messaging/email/mailer'
import { VerifyContent } from '@/app/(auth)/verify/verify-content'

Expand Down
2 changes: 1 addition & 1 deletion apps/sim/app/(landing)/components/legal-layout.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { getNavBlogPosts } from '@/lib/blog/registry'
import { isHosted } from '@/lib/core/config/feature-flags'
import { isHosted } from '@/lib/core/config/env-flags'
import Footer from '@/app/(landing)/components/footer/footer'
import Navbar from '@/app/(landing)/components/navbar/navbar'

Expand Down
2 changes: 1 addition & 1 deletion apps/sim/app/(landing)/contact/page.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { Metadata } from 'next'
import { getNavBlogPosts } from '@/lib/blog/registry'
import { isHosted } from '@/lib/core/config/feature-flags'
import { isHosted } from '@/lib/core/config/env-flags'
import { SITE_URL } from '@/lib/core/utils/urls'
import { ContactForm } from '@/app/(landing)/components/contact/contact-form'
import Footer from '@/app/(landing)/components/footer/footer'
Expand Down
2 changes: 1 addition & 1 deletion apps/sim/app/api/auth/[...all]/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ vi.mock('@/lib/auth/anonymous', () => ({
createAnonymousSession: handlerMocks.createAnonymousSession,
}))

vi.mock('@/lib/core/config/feature-flags', () => ({
vi.mock('@/lib/core/config/env-flags', () => ({
get isAuthDisabled() {
return handlerMocks.isAuthDisabled
},
Expand Down
2 changes: 1 addition & 1 deletion apps/sim/app/api/auth/[...all]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { toNextJsHandler } from 'better-auth/next-js'
import { type NextRequest, NextResponse } from 'next/server'
import { auth } from '@/lib/auth'
import { createAnonymousSession, ensureAnonymousUserExists } from '@/lib/auth/anonymous'
import { isAuthDisabled } from '@/lib/core/config/feature-flags'
import { isAuthDisabled } from '@/lib/core/config/env-flags'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'

export const dynamic = 'force-dynamic'
Expand Down
2 changes: 1 addition & 1 deletion apps/sim/app/api/auth/providers/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { NextRequest } from 'next/server'
import { NextResponse } from 'next/server'
import { getAuthProvidersContract } from '@/lib/api/contracts/auth'
import { parseRequest } from '@/lib/api/server'
import { isRegistrationDisabled } from '@/lib/core/config/feature-flags'
import { isRegistrationDisabled } from '@/lib/core/config/env-flags'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
import { getOAuthProviderStatus } from '@/app/(auth)/components/oauth-provider-checker'

Expand Down
2 changes: 1 addition & 1 deletion apps/sim/app/api/auth/socket-token/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { toError } from '@sim/utils/errors'
import { headers } from 'next/headers'
import { type NextRequest, NextResponse } from 'next/server'
import { auth } from '@/lib/auth'
import { isAuthDisabled } from '@/lib/core/config/feature-flags'
import { isAuthDisabled } from '@/lib/core/config/env-flags'
import { enforceIpRateLimit } from '@/lib/core/rate-limiter'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'

Expand Down
2 changes: 1 addition & 1 deletion apps/sim/app/api/billing/switch-plan/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
hasUsableSubscriptionStatus,
isOrgScopedSubscription,
} from '@/lib/billing/subscriptions/utils'
import { isBillingEnabled } from '@/lib/core/config/feature-flags'
import { isBillingEnabled } from '@/lib/core/config/env-flags'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
import { captureServerEvent } from '@/lib/posthog/server'

Expand Down
2 changes: 1 addition & 1 deletion apps/sim/app/api/billing/update-cost/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ vi.mock('@/lib/billing/threshold-billing', () => ({
checkAndBillOverageThreshold: mockCheckAndBillOverageThreshold,
}))

vi.mock('@/lib/core/config/feature-flags', () => ({
vi.mock('@/lib/core/config/env-flags', () => ({
isBillingEnabled: true,
}))

Expand Down
2 changes: 1 addition & 1 deletion apps/sim/app/api/billing/update-cost/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { TraceAttr } from '@/lib/copilot/generated/trace-attributes-v1'
import { TraceSpan } from '@/lib/copilot/generated/trace-spans-v1'
import { checkInternalApiKey } from '@/lib/copilot/request/http'
import { withIncomingGoSpan } from '@/lib/copilot/request/otel'
import { isBillingEnabled } from '@/lib/core/config/feature-flags'
import { isBillingEnabled } from '@/lib/core/config/env-flags'
import { generateRequestId } from '@/lib/core/utils/request'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'

Expand Down
2 changes: 1 addition & 1 deletion apps/sim/app/api/chat/manage/[id]/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const mockNotifySocketDeploymentChanged =
workflowsOrchestrationMockFns.mockNotifySocketDeploymentChanged

vi.mock('@sim/audit', () => auditMock)
vi.mock('@/lib/core/config/feature-flags', () => ({
vi.mock('@/lib/core/config/env-flags', () => ({
isDev: true,
isHosted: false,
isProd: false,
Expand Down
2 changes: 1 addition & 1 deletion apps/sim/app/api/chat/manage/[id]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type { NextRequest } from 'next/server'
import { chatIdParamsSchema, updateChatContract } from '@/lib/api/contracts/chats'
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
import { getSession } from '@/lib/auth'
import { isDev } from '@/lib/core/config/feature-flags'
import { isDev } from '@/lib/core/config/env-flags'
import { encryptSecret } from '@/lib/core/security/encryption'
import { getEmailDomain } from '@/lib/core/utils/urls'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
Expand Down
2 changes: 1 addition & 1 deletion apps/sim/app/api/chat/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ vi.mock('@/lib/core/security/deployment', () => ({
isEmailAllowed: mockIsEmailAllowed,
}))

vi.mock('@/lib/core/config/feature-flags', () => ({
vi.mock('@/lib/core/config/env-flags', () => ({
isDev: true,
isProd: false,
get isBillingEnabled() {
Expand Down
2 changes: 1 addition & 1 deletion apps/sim/app/api/chat/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { and, eq, isNull } from 'drizzle-orm'
import type { NextRequest, NextResponse } from 'next/server'
import { isWorkspaceApiExecutionEntitled } from '@/lib/billing/core/api-access'
import { getEnv } from '@/lib/core/config/env'
import { isBillingEnabled, isFreeApiDeploymentGateEnabled } from '@/lib/core/config/feature-flags'
import { isBillingEnabled, isFreeApiDeploymentGateEnabled } from '@/lib/core/config/env-flags'
import type { TokenBucketConfig } from '@/lib/core/rate-limiter'
import { RateLimiter } from '@/lib/core/rate-limiter'
import {
Expand Down
2 changes: 1 addition & 1 deletion apps/sim/app/api/copilot/api-keys/validate/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ vi.mock('@/lib/copilot/request/otel', () => ({
) => fn({ setAttribute: vi.fn(), setAttributes: vi.fn() }),
}))

vi.mock('@/lib/core/config/feature-flags', () => ({
vi.mock('@/lib/core/config/env-flags', () => ({
get isHosted() {
return mockFlags.isHosted
},
Expand Down
2 changes: 1 addition & 1 deletion apps/sim/app/api/copilot/api-keys/validate/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { TraceAttr } from '@/lib/copilot/generated/trace-attributes-v1'
import { TraceSpan } from '@/lib/copilot/generated/trace-spans-v1'
import { checkInternalApiKey } from '@/lib/copilot/request/http'
import { withIncomingGoSpan } from '@/lib/copilot/request/otel'
import { isHosted } from '@/lib/core/config/feature-flags'
import { isHosted } from '@/lib/core/config/env-flags'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'

const logger = createLogger('CopilotApiKeysValidate')
Expand Down
Loading
Loading