From 4157732054b101951e55f6fb423db0b132fad7f5 Mon Sep 17 00:00:00 2001 From: mantrakp04 Date: Thu, 30 Apr 2026 15:57:31 -0700 Subject: [PATCH] Enhance feature flag evaluation and configuration handling - Introduced verification for primary email in the feature flag evaluation context to ensure only verified emails are used. - Updated the evaluation logic to handle distinct IDs more flexibly, allowing for better fallback mechanisms. - Improved the PATCH route for configuration overrides by implementing a new method to merge old and new configurations, ensuring validation before writing. - Added comprehensive tests to validate the new feature flag evaluation scenarios, including handling of unverified emails and dependency cycles. --- .../latest/feature-flags/evaluate/route.ts | 7 +- .../config/override/[level]/route.tsx | 17 ++- .../feature-flags/[flagId]/page-client.tsx | 25 +++- .../endpoints/api/v1/feature-flags.test.ts | 102 +++++++++++++++- claude/CLAUDE-KNOWLEDGE.md | 3 + .../demo/src/app/feature-flags-demo/page.tsx | 17 ++- .../src/config/schema-fuzzer.test.ts | 20 +++- packages/stack-shared/src/config/schema.ts | 111 ++++++++++++++++++ .../src/feature-flags/evaluator.ts | 45 ++++++- .../client-app-feature-flags.test.tsx | 7 +- .../apps/implementations/client-app-impl.ts | 2 +- 11 files changed, 336 insertions(+), 20 deletions(-) diff --git a/apps/backend/src/app/api/latest/feature-flags/evaluate/route.ts b/apps/backend/src/app/api/latest/feature-flags/evaluate/route.ts index fdf458017..fe0403b2a 100644 --- a/apps/backend/src/app/api/latest/feature-flags/evaluate/route.ts +++ b/apps/backend/src/app/api/latest/feature-flags/evaluate/route.ts @@ -49,6 +49,7 @@ export const POST = createSmartRouteHandler({ const config: FeatureFlagsConfig = auth.tenancy.config.featureFlags; const callerCanSupplyTargetingContext = auth.type !== "client"; + const verifiedPrimaryEmail = auth.user?.primary_email_verified ? auth.user.primary_email ?? undefined : undefined; const evalContext: EvalContext = callerCanSupplyTargetingContext ? { distinctId: body.distinct_id ?? body.user_id ?? auth.user?.id, userId: body.user_id ?? auth.user?.id, @@ -58,13 +59,13 @@ export const POST = createSmartRouteHandler({ context: body.context, cohorts: body.cohorts, } : { - distinctId: auth.user?.id, + distinctId: body.distinct_id ?? auth.user?.id, userId: auth.user?.id, user: auth.user ? { id: auth.user.id, - primary_email: auth.user.primary_email, + primary_email: verifiedPrimaryEmail, primary_email_verified: auth.user.primary_email_verified, - email: auth.user.primary_email, + email: verifiedPrimaryEmail, } : undefined, }; diff --git a/apps/backend/src/app/api/latest/internal/config/override/[level]/route.tsx b/apps/backend/src/app/api/latest/internal/config/override/[level]/route.tsx index 1f6ba5055..072f2ac63 100644 --- a/apps/backend/src/app/api/latest/internal/config/override/[level]/route.tsx +++ b/apps/backend/src/app/api/latest/internal/config/override/[level]/route.tsx @@ -18,6 +18,7 @@ import { LOCAL_EMULATOR_ENV_CONFIG_BLOCKED_MESSAGE, isLocalEmulatorProject } fro import { globalPrismaClient, rawQuery } from "@/prisma-client"; import { createSmartRouteHandler } from "@/route-handlers/smart-route-handler"; import { branchConfigSchema, environmentConfigSchema, getConfigOverrideErrors, migrateConfigOverride, projectConfigSchema } from "@stackframe/stack-shared/dist/config/schema"; +import { override } from "@stackframe/stack-shared/dist/config/format"; import { adaptSchema, branchConfigSourceSchema, serverOrHigherAuthTypeSchema, yupNumber, yupObject, yupString } from "@stackframe/stack-shared/dist/schema-fields"; import { StackAssertionError, StatusError, captureError } from "@stackframe/stack-shared/dist/utils/errors"; import * as yup from "yup"; @@ -317,10 +318,22 @@ export const PATCH = createSmartRouteHandler({ const levelConfig = levelConfigs[req.params.level]; const parsedConfig = await parseAndValidateConfig(req.body.config_override_string, levelConfig); - const newConfig = await levelConfig.override({ + const oldConfig = await levelConfig.get({ projectId: req.auth.tenancy.project.id, branchId: req.auth.tenancy.branchId, - config: parsedConfig, + }); + const newConfig = override(oldConfig, parsedConfig); + + await assertConfigValidBeforeWrite(levelConfig, { + projectId: req.auth.tenancy.project.id, + branchId: req.auth.tenancy.branchId, + config: newConfig, + }); + + await levelConfig.set({ + projectId: req.auth.tenancy.project.id, + branchId: req.auth.tenancy.branchId, + config: newConfig, }); await warnOnValidationFailure(levelConfig, { diff --git a/apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/feature-flags/[flagId]/page-client.tsx b/apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/feature-flags/[flagId]/page-client.tsx index 39f6cc263..e06dc1d09 100644 --- a/apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/feature-flags/[flagId]/page-client.tsx +++ b/apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/feature-flags/[flagId]/page-client.tsx @@ -292,11 +292,34 @@ export default function PageClient() { // Live evaluation against current config + the JSON-edited debug context. const debugResult = (() => { + const parsedVariants = tryParseJson(variantsJson); + if (!parsedVariants.ok) return { error: `Invalid variants JSON: ${parsedVariants.error}` }; + const variants = validateVariants(parsedVariants.value); + if (!variants.ok) return { error: variants.error }; + + const parsedRules = tryParseJson(rulesJson); + if (!parsedRules.ok) return { error: `Invalid rules JSON: ${parsedRules.error}` }; + const rules = validateRules(parsedRules.value); + if (!rules.ok) return { error: rules.error }; + const parsed = tryParseJson(debugContextJson); if (!parsed.ok) return { error: parsed.error }; const ctx = validateEvalContext(parsed.value); if (!ctx.ok) return { error: ctx.error }; - const cfg: FeatureFlagsConfig = config.featureFlags; + + const cfg: FeatureFlagsConfig = { + ...config.featureFlags, + flags: { + ...config.featureFlags.flags, + [flagId]: { + ...flag, + description: description.trim() || undefined, + defaultVariantKey: defaultVariantKey.trim() || undefined, + variants: variants.value, + rules: rules.value, + }, + }, + }; return { result: evaluateFlag(flagId, cfg, ctx.value) }; })(); diff --git a/apps/e2e/tests/backend/endpoints/api/v1/feature-flags.test.ts b/apps/e2e/tests/backend/endpoints/api/v1/feature-flags.test.ts index 7e5839e65..7e8268624 100644 --- a/apps/e2e/tests/backend/endpoints/api/v1/feature-flags.test.ts +++ b/apps/e2e/tests/backend/endpoints/api/v1/feature-flags.test.ts @@ -1,7 +1,8 @@ import { it } from "../../../../helpers"; import { evaluateFlag } from "@stackframe/stack-shared/dist/feature-flags/evaluator"; import type { FeatureFlagsConfig } from "@stackframe/stack-shared/dist/feature-flags/types"; -import { Auth, Project, niceBackendFetch } from "../../../backend-helpers"; +import { Auth, Project, backendContext, createMailbox, niceBackendFetch } from "../../../backend-helpers"; +import { randomUUID } from "node:crypto"; async function setupProjectWithFlags(featureFlags: FeatureFlagsConfig) { await Project.createAndSwitch(); @@ -229,8 +230,48 @@ it("/feature-flags/evaluate scopes to flag_keys when provided", async ({ expect expect(Object.keys(response.body.results)).toEqual(["f1"]); }); -it("/feature-flags/evaluate falls back to the authenticated user id when distinct_id is omitted", async ({ expect }) => { +it("/internal/config/override PATCH rejects feature flag updates that make the merged config invalid", async ({ expect }) => { await setupProjectWithFlags({ + flags: { + "flag-a": { key: "a", type: "boolean", enabled: true, defaultVariantKey: "off", + variants: { off: { value: false } }, rules: {} }, + "flag-b": { key: "b", type: "boolean", enabled: true, defaultVariantKey: "off", + variants: { off: { value: false } }, rules: {} }, + }, + }); + + const response = await niceBackendFetch("/api/latest/internal/config/override/branch", { + method: "PATCH", + accessType: "admin", + body: { + config_override_string: JSON.stringify({ + "featureFlags.flags.flag-b": { + key: "a", + type: "boolean", + enabled: true, + defaultVariantKey: "off", + variants: { off: { value: false } }, + rules: {}, + }, + }), + }, + }); + expect(response.status).toBe(400); + + const stillValidResponse = await niceBackendFetch("/api/latest/feature-flags/evaluate", { + method: "POST", + accessType: "client", + body: { flag_keys: ["b"] }, + }); + expect(stillValidResponse.body.results.b).toMatchObject({ + flag_key: "b", + variant_key: "off", + value: false, + }); +}); + +it("/feature-flags/evaluate uses client distinct_id when provided and auth user id when omitted", async ({ expect }) => { + const featureFlags: FeatureFlagsConfig = { flags: { "rollout-flag": { key: "rollout", @@ -246,7 +287,8 @@ it("/feature-flags/evaluate falls back to the authenticated user id when distinc }, }, }, - }); + }; + await setupProjectWithFlags(featureFlags); const { userId } = await Auth.Anonymous.signUp(); const omittedDistinctId = await niceBackendFetch("/api/latest/feature-flags/evaluate", { @@ -257,13 +299,20 @@ it("/feature-flags/evaluate falls back to the authenticated user id when distinc const explicitUserId = await niceBackendFetch("/api/latest/feature-flags/evaluate", { method: "POST", accessType: "client", - body: { distinct_id: "caller-supplied-id-is-ignored", flag_keys: ["rollout"] }, + body: { distinct_id: "caller-supplied-id-is-used", flag_keys: ["rollout"] }, }); + const omittedExpected = evaluateFlag("rollout-flag", featureFlags, { distinctId: userId, userId }); + const explicitExpected = evaluateFlag("rollout-flag", featureFlags, { distinctId: "caller-supplied-id-is-used", userId }); expect(omittedDistinctId.body.results.rollout).toMatchObject({ flag_key: "rollout", - variant_key: explicitUserId.body.results.rollout.variant_key, - value: explicitUserId.body.results.rollout.value, + variant_key: omittedExpected.variantKey, + value: omittedExpected.value, + }); + expect(explicitUserId.body.results.rollout).toMatchObject({ + flag_key: "rollout", + variant_key: explicitExpected.variantKey, + value: explicitExpected.value, }); }); @@ -323,6 +372,47 @@ it("/feature-flags/evaluate ignores caller-supplied targeting attributes for cli }); }); +it("/feature-flags/evaluate does not expose unverified primary email as trusted client targeting context", async ({ expect }) => { + await setupProjectWithFlags({ + flags: { + "internal-tools": { + key: "internal-tools", + type: "boolean", + enabled: true, + defaultVariantKey: "off", + variants: { on: { value: true }, off: { value: false } }, + rules: { + employee: { + priority: 0, + enabled: true, + rolloutPercentage: 100, + variantKey: "on", + conditions: { + email: { attribute: "user.email", operator: "regex", value: "@stack-auth\\.com$" }, + }, + }, + }, + }, + }, + }); + + backendContext.set({ mailbox: createMailbox(`unverified-feature-flag-user--${randomUUID()}@stack-auth.com`) }); + await Auth.Password.signUpWithEmail({ noWaitForEmail: true }); + + const response = await niceBackendFetch("/api/latest/feature-flags/evaluate", { + method: "POST", + accessType: "client", + body: { + flag_keys: ["internal-tools"], + }, + }); + expect(response.body.results["internal-tools"]).toMatchObject({ + variant_key: "off", + value: false, + reason: "default", + }); +}); + it("/feature-flags/evaluate returns missing for requested unknown flag keys", async ({ expect }) => { await setupProjectWithFlags({ flags: { diff --git a/claude/CLAUDE-KNOWLEDGE.md b/claude/CLAUDE-KNOWLEDGE.md index cf466229d..4edf22fd1 100644 --- a/claude/CLAUDE-KNOWLEDGE.md +++ b/claude/CLAUDE-KNOWLEDGE.md @@ -392,3 +392,6 @@ A: Use a strict root `postinstall` script that rewrites only Next `>=16` app-pag Q: Why can Turbo-pruned Docker builds fail with `Cannot find module /app/scripts/postinstall-patch-next-async-debug-info.mjs` during `pnpm install`? A: In pruned builder stages, we copy `/app/out/json` and run `pnpm install` before copying `/app/out/full`. The root `package.json` still runs `postinstall: node ./scripts/postinstall-patch-next-async-debug-info.mjs`, but that script is not present yet. Fix by copying `scripts/postinstall-patch-next-async-debug-info.mjs` into the builder stage before `pnpm install` (for all Dockerfiles using the prune pattern). + +Q: Why can feature flag E2E tests fail with `The key "featureFlags" is not valid` on a branch that adds the schema? +A: The backend routes import generated package output from `@stackframe/stack-shared/dist`. If the dev package generator has not rebuilt that dist after adding `featureFlags` to the schema/app ids, backend E2E setup calls like `Project.updatePushedConfig({ featureFlags: ... })` still run against the old schema and reject the key before the test logic starts. diff --git a/examples/demo/src/app/feature-flags-demo/page.tsx b/examples/demo/src/app/feature-flags-demo/page.tsx index e93a5a823..078fca5a7 100644 --- a/examples/demo/src/app/feature-flags-demo/page.tsx +++ b/examples/demo/src/app/feature-flags-demo/page.tsx @@ -16,14 +16,25 @@ const apiUrl = process.env.NEXT_PUBLIC_STACK_API_URL; const projectId = process.env.NEXT_PUBLIC_STACK_PROJECT_ID; const publishableClientKey = process.env.NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY; +function getRequiredPublicEnv(name: string, value: string | undefined): string { + if (value == null || value.length === 0) { + throw new Error(`Expected ${name} to be configured for the feature flags demo`); + } + return value; +} + +function getDemoApiUrl(): string { + return getRequiredPublicEnv("NEXT_PUBLIC_STACK_API_URL", apiUrl).replace(/\/+$/, ""); +} + async function evaluateFlags(body: Record): Promise> { - const res = await fetch(`${apiUrl}/api/latest/feature-flags/evaluate`, { + const res = await fetch(`${getDemoApiUrl()}/api/latest/feature-flags/evaluate`, { method: "POST", headers: { "content-type": "application/json", "x-stack-access-type": "client", - "x-stack-project-id": projectId ?? "", - "x-stack-publishable-client-key": publishableClientKey ?? "", + "x-stack-project-id": getRequiredPublicEnv("NEXT_PUBLIC_STACK_PROJECT_ID", projectId), + "x-stack-publishable-client-key": getRequiredPublicEnv("NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY", publishableClientKey), }, body: JSON.stringify(body), }); diff --git a/packages/stack-shared/src/config/schema-fuzzer.test.ts b/packages/stack-shared/src/config/schema-fuzzer.test.ts index 56b270a4b..ca1fe6b58 100644 --- a/packages/stack-shared/src/config/schema-fuzzer.test.ts +++ b/packages/stack-shared/src/config/schema-fuzzer.test.ts @@ -205,7 +205,7 @@ const branchSchemaFuzzerConfig = [{ "some-flag-id": [{ key: ["new-checkout"], description: ["A flag for the new checkout"], - type: ["boolean", "multivariate", "json", "numeric", "string"] as const, + type: ["boolean"] as const, enabled: [true, false], killSwitch: [true, false], tags: [{ "beta": [true], "internal": [true] }] as const, @@ -250,6 +250,24 @@ const branchSchemaFuzzerConfig = [{ }], }], }], + "another-flag-id": [{ + key: ["dependency-flag"], + description: [undefined], + type: ["boolean"] as const, + enabled: [true], + killSwitch: [false], + tags: [undefined], + ownerUserId: [undefined], + dependsOn: [undefined], + holdoutId: [undefined], + defaultVariantKey: ["on"], + variants: [{ + "on": [{ + value: [true], + }], + }], + rules: [{}], + }], }], holdouts: [{ "some-holdout-id": [{ diff --git a/packages/stack-shared/src/config/schema.ts b/packages/stack-shared/src/config/schema.ts index cf8a7c987..4fb19bfe5 100644 --- a/packages/stack-shared/src/config/schema.ts +++ b/packages/stack-shared/src/config/schema.ts @@ -218,10 +218,14 @@ const featureFlagStickyBy = ["userId", "teamId", "distinctId"] as const; const featureFlagTypes = ["boolean", "multivariate", "json", "numeric", "string"] as const; type FeatureFlagSchemaValue = { + holdouts?: Record, flags?: Record, defaultVariantKey?: string, + dependsOn?: string, + holdoutId?: string, rules?: Record, @@ -229,6 +233,43 @@ type FeatureFlagSchemaValue = { }>, }; +function isJsonCompatibleFeatureFlagValue(value: unknown): boolean { + if (value === null) return true; + if (typeof value === "string" || typeof value === "boolean") return true; + if (typeof value === "number") return Number.isFinite(value); + if (Array.isArray(value)) { + return value.every((item) => item !== undefined && isJsonCompatibleFeatureFlagValue(item)); + } + if (value !== undefined && typeof value === "object") { + return Object.values(value).every((nestedValue) => nestedValue !== undefined && isJsonCompatibleFeatureFlagValue(nestedValue)); + } + return false; +} + +function getFeatureFlagVariantValueError(flagType: string | undefined, variantValue: unknown): string | undefined { + switch (flagType) { + case "boolean": { + return typeof variantValue === "boolean" ? undefined : "boolean flag variants must have boolean values"; + } + case "numeric": { + return typeof variantValue === "number" && Number.isFinite(variantValue) ? undefined : "numeric flag variants must have finite number values"; + } + case "string": { + return typeof variantValue === "string" ? undefined : "string flag variants must have string values"; + } + case "json": { + return isJsonCompatibleFeatureFlagValue(variantValue) ? undefined : "json flag variants must have JSON-compatible values"; + } + case "multivariate": + case undefined: { + return undefined; + } + default: { + return `unknown flag type "${flagType}"`; + } + } +} + export const branchFeatureFlagsSchema = yupObject({ flags: yupRecord( userSpecifiedIdSchema("flagId"), @@ -356,6 +397,8 @@ export const branchFeatureFlagsSchema = yupObject({ 'Feature flag variant references must point to existing variants', function(this: yup.TestContext, value: FeatureFlagSchemaValue | undefined) { if (!value?.flags) return true; + const flagIds = new Set(Object.keys(value.flags)); + const holdoutIds = new Set(Object.keys(value.holdouts ?? {})); for (const [flagId, flag] of Object.entries(value.flags)) { const variants = flag.variants ?? {}; const variantKeys = new Set(Object.keys(variants)); @@ -365,6 +408,28 @@ export const branchFeatureFlagsSchema = yupObject({ path: `${this.path}.flags.${flagId}.defaultVariantKey`, }); } + if (flag.dependsOn !== undefined && !flagIds.has(flag.dependsOn)) { + return this.createError({ + message: `Feature flag "${flagId}" dependsOn "${flag.dependsOn}" does not reference an existing flag`, + path: `${this.path}.flags.${flagId}.dependsOn`, + }); + } + if (flag.holdoutId !== undefined && !holdoutIds.has(flag.holdoutId)) { + return this.createError({ + message: `Feature flag "${flagId}" holdoutId "${flag.holdoutId}" does not reference an existing holdout`, + path: `${this.path}.flags.${flagId}.holdoutId`, + }); + } + for (const [variantKey, variant] of Object.entries(variants)) { + const variantValue = variant != null && typeof variant === "object" && !Array.isArray(variant) ? (variant as { value?: unknown }).value : undefined; + const variantValueError = getFeatureFlagVariantValueError(flag.type, variantValue); + if (variantValueError !== undefined) { + return this.createError({ + message: `Feature flag "${flagId}" variant "${variantKey}" is invalid: ${variantValueError}`, + path: `${this.path}.flags.${flagId}.variants.${variantKey}.value`, + }); + } + } const rules = flag.rules ?? {}; for (const [ruleId, rule] of Object.entries(rules)) { if (rule.variantKey !== undefined && !variantKeys.has(rule.variantKey)) { @@ -388,6 +453,52 @@ export const branchFeatureFlagsSchema = yupObject({ ); // --- END Feature Flags Schema --- +import.meta.vitest?.test("branchFeatureFlagsSchema rejects missing dependency and holdout references", async ({ expect }) => { + await expect(branchFeatureFlagsSchema.validate({ + flags: { + child: { + key: "child", + type: "boolean", + dependsOn: "missing-gate", + holdoutId: "missing-holdout", + defaultVariantKey: "off", + variants: { + off: { value: false }, + }, + }, + }, + holdouts: {}, + })).rejects.toThrow('Feature flag "child" dependsOn "missing-gate" does not reference an existing flag'); +}); + +import.meta.vitest?.test("branchFeatureFlagsSchema validates variant values against flag type", async ({ expect }) => { + await expect(branchFeatureFlagsSchema.validate({ + flags: { + flag: { + key: "flag", + type: "boolean", + defaultVariantKey: "on", + variants: { + on: { value: "true" }, + }, + }, + }, + })).rejects.toThrow('Feature flag "flag" variant "on" is invalid: boolean flag variants must have boolean values'); + + await expect(branchFeatureFlagsSchema.validate({ + flags: { + flag: { + key: "flag", + type: "numeric", + defaultVariantKey: "one", + variants: { + one: { value: 1 }, + }, + }, + }, + })).resolves.toBeDefined(); +}); + export const branchConfigSchema = canNoLongerBeOverridden(projectConfigSchema, [ "sourceOfTruth", diff --git a/packages/stack-shared/src/feature-flags/evaluator.ts b/packages/stack-shared/src/feature-flags/evaluator.ts index 330d048c0..d3e4360d9 100644 --- a/packages/stack-shared/src/feature-flags/evaluator.ts +++ b/packages/stack-shared/src/feature-flags/evaluator.ts @@ -175,7 +175,7 @@ function applyOperator( } case "in_cohort": { if (typeof expected !== "string") return false; - return Boolean(cohorts?.[expected]); + return cohorts !== undefined && Object.prototype.hasOwnProperty.call(cohorts, expected) && cohorts[expected] === true; } } } @@ -258,6 +258,16 @@ export function evaluateFlag( context: EvalContext, seenFlags: ReadonlySet = new Set(), ): EvalResult { + if (seenFlags.has(flagKey)) { + const flag = config.flags?.[flagKey]; + return { + flagKey, + variantKey: flag?.defaultVariantKey, + value: variantValue(flag ?? {}, flag?.defaultVariantKey), + reason: "cycle", + }; + } + const flag = config.flags?.[flagKey]; if (!flag) { return { flagKey, variantKey: undefined, value: undefined, reason: "missing" }; @@ -275,8 +285,8 @@ export function evaluateFlag( if (flag.enabled === false) return defaultResult("disabled"); if (flag.dependsOn) { - if (seenFlags.has(flag.dependsOn)) return defaultResult("cycle"); const dep = evaluateFlag(flag.dependsOn, config, context, new Set([...seenFlags, flagKey])); + if (dep.reason === "cycle") return defaultResult("cycle"); if (!isTruthyFlagValue(dep.value)) return defaultResult("dep_unmet"); } @@ -513,6 +523,21 @@ import.meta.vitest?.test("dependency cycles are broken safely", ({ expect }) => expect(r.value).toBe(false); }); +import.meta.vitest?.test("dependency cycles do not pass through truthy defaults", ({ expect }) => { + const config: FeatureFlagsConfig = { + flags: { + a: { key: "a", type: "boolean", enabled: true, defaultVariantKey: "on", dependsOn: "b", + variants: { on: { value: true }, off: { value: false } }, + rules: { all: { variantKey: "off" } } }, + b: { key: "b", type: "boolean", enabled: true, defaultVariantKey: "on", dependsOn: "a", + variants: { on: { value: true }, off: { value: false } } }, + }, + }; + const r = evaluateFlag("a", config, { distinctId: "u" }); + expect(r.reason).toBe("cycle"); + expect(r.value).toBe(true); +}); + import.meta.vitest?.test("holdout excludes a slice from any variant", ({ expect }) => { const config: FeatureFlagsConfig = { flags: { @@ -583,6 +608,22 @@ import.meta.vitest?.test("in_cohort matches cohort membership in context", ({ ex expect(evaluateFlag("f", config, { distinctId: "u", cohorts: {} }).value).toBe(false); }); +import.meta.vitest?.test("in_cohort ignores inherited object properties", ({ expect }) => { + const config: FeatureFlagsConfig = { + flags: { + f: { + key: "f", type: "boolean", enabled: true, defaultVariantKey: "off", + variants: { on: { value: true }, off: { value: false } }, + rules: { r: { + priority: 0, rolloutPercentage: 100, variantKey: "on", + conditions: { c: { attribute: "user.id", operator: "in_cohort", value: "toString" } }, + } }, + }, + }, + }; + expect(evaluateFlag("f", config, { distinctId: "u", cohorts: {} }).value).toBe(false); +}); + import.meta.vitest?.test("evaluation is sticky for the same distinctId", ({ expect }) => { const config: FeatureFlagsConfig = { flags: { diff --git a/packages/template/src/lib/stack-app/apps/implementations/client-app-feature-flags.test.tsx b/packages/template/src/lib/stack-app/apps/implementations/client-app-feature-flags.test.tsx index 2eecae673..fe601c9ec 100644 --- a/packages/template/src/lib/stack-app/apps/implementations/client-app-feature-flags.test.tsx +++ b/packages/template/src/lib/stack-app/apps/implementations/client-app-feature-flags.test.tsx @@ -88,7 +88,9 @@ function createInterface( projectId, publishableClientKey: "pck_test", }); - vi.spyOn(iface, "evaluateFeatureFlags").mockImplementation(async (body) => await evaluateFeatureFlags(body)); + Object.assign(iface, { + evaluateFeatureFlags: async (body: FeatureFlagEvaluateRequest) => await evaluateFeatureFlags(body), + }); return iface; } @@ -177,6 +179,9 @@ describe("StackClientApp feature flag evaluation", () => { await expect(app.getFeatureFlags(["present", "omitted"])).rejects.toThrow( "Feature flag evaluate response did not include requested key omitted", ); + await expect(app.getFeatureFlags(["present", "toString"])).rejects.toThrow( + "Feature flag evaluate response did not include requested key toString", + ); }); it("evaluates feature flags through React-like hooks and caches equivalent ordered batches", async () => { diff --git a/packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts b/packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts index 907284fce..56e8a7838 100644 --- a/packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts +++ b/packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts @@ -2731,7 +2731,7 @@ export class _StackClientAppImplIncomplete [key, this._featureFlagResultFromCrud(result)]), ); for (const key of new Set(keys)) { - if (!(key in results)) { + if (!Object.prototype.hasOwnProperty.call(results, key)) { throwErr(`Feature flag evaluate response did not include requested key ${key}`); } }