Skip to content

feat: Evaluate custom targets in the feature flags runtime client - #1674

Open
stanleyphu wants to merge 2 commits into
mainfrom
feat/feature-flags-custom-target-evaluation
Open

feat: Evaluate custom targets in the feature flags runtime client#1674
stanleyphu wants to merge 2 commits into
mainfrom
feat/feature-flags-custom-target-evaluation

Conversation

@stanleyphu

@stanleyphu stanleyphu commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Teaches the feature flags runtime client to evaluate the targets.custom_targets collection that the API now includes in the GET /sdk/feature-flags poll payload behind the feature-flags-custom-targets rollout flag (workos/workos#68135). This is stage 4 of the Custom Targeting Hilltop rollout plan: ship the runtime contract before typed target management or docs.

Typed evaluation context

isEnabled / getAllFlags now accept the Hilltop's direct target-type context map alongside the existing legacy shape:

// Legacy shape — still supported, normalized internally
client.isEnabled('new-dashboard', { userId: 'user_abc', organizationId: 'org_abc' });

// Typed shape — built-ins and custom types are peers
client.isEnabled('new-dashboard', {
  user: { id: 'user_abc' },
  workspace: { id: 'ws_123' },
});

Per the Hilltop's pre-commitments:

  • Hybrid contexts (a legacy key alongside a typed resource entry in one call) are rejected rather than given an invented precedence: a warning is logged and no targets match. This is enforced at runtime — the union type does not reject hybrids at compile time (TypedEvaluationContext's index signature makes any key acceptable to the union). Shape detection is by value, not key presence: unset keys and scalar extra fields on a legacy context are ignored, preserving today's behavior for callers passing wider objects.
  • Evaluation never throws on invalid context: invalid type keys and IDs (validated with the same rules as the API) log a warning and evaluate as non-matching.
  • Target IDs are compared exactly (case-sensitive, no normalization).

Evaluation semantics aligned with the Hilltop

  • Evaluation is enable-only: any enabled target matching the context turns the flag on, with no precedence between target types.
  • A target with enabled: false is treated as not present (falls through to the flag default) instead of forcing the flag off. This field value is reserved for future disabled overrides, per the Hilltop's Forward compatibility for disabled overrides. The API only ever writes enabled: true today, so this is not an observable behavior change — but it does replace the previous user-over-organization short-circuit, which only mattered for enabled: false rows that cannot exist.

Change detection

hasEntryChanged now also diffs custom_targets, so a poll cycle where only a custom rule changed emits the change event.

Compatibility

  • targets.custom_targets is optional in the payload types: polls against servers with the rollout flag off (or older servers) behave exactly as before.
  • Until this version is adopted, custom-only flags evaluate to their default on older SDKs — which is why the API keeps the payload gated per team.

Test plan

  • npx jest src/feature-flags — 93 tests passing, including new coverage for: exact custom matches, built-in types via the typed form, enabled: false treated as absent (including the default-true proof), hybrid-context rejection, unset-key and scalar-extra-field handling, once-per-call warnings in getAllFlags, invalid type/ID warnings, malformed context safety, and a change event fired when only custom targets differ.
  • npm run build (tsdown + attw + publint) and npx eslint src/feature-flags clean.
  • Manual: point the client at an environment with the feature-flags-custom-targets rollout flag enabled, add a custom rule in the dashboard, and confirm isEnabled(slug, { <type>: { id } }) flips accordingly.

🤖 Generated with Claude Code

@stanleyphu
stanleyphu marked this pull request as ready for review August 13, 2026 20:16
@stanleyphu
stanleyphu requested review from a team as code owners August 13, 2026 20:16
@stanleyphu
stanleyphu requested a review from nicknisi August 13, 2026 20:16
@greptile-apps

greptile-apps Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR extends local feature-flag evaluation to support typed built-in and custom target contexts while retaining legacy context support.

  • Adds context normalization, validation, warning behavior, and enable-only matching across target types.
  • Extends polling payload types and change detection for optional custom targets.
  • Adds evaluator and runtime-client coverage for matching, malformed contexts, compatibility, and change events.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/feature-flags/evaluator.ts Adds safe context normalization and enable-only evaluation for built-in and custom targets.
src/feature-flags/interfaces/evaluation-context.interface.ts Expands the public evaluation context contract to include typed resource maps alongside the legacy shape.
src/feature-flags/interfaces/flag-poll-response.interface.ts Adds the optional custom-target collection to feature-flag poll entries.
src/feature-flags/runtime-client.ts Passes the configured logger into evaluation and detects custom-target changes between polls.
src/feature-flags/evaluator.spec.ts Covers typed and legacy evaluation, custom matching, invalid contexts, and enable-only semantics.
src/feature-flags/runtime-client.spec.ts Covers typed built-in evaluation and custom-target-only change events.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  App["Application"] --> Client["FeatureFlagsRuntimeClient"]
  Client --> Normalize["Normalize legacy or typed context"]
  Normalize --> Evaluate["Evaluator"]
  Poll["GET /sdk/feature-flags"] --> Store["InMemoryStore"]
  Store --> Evaluate
  Evaluate --> Builtins["User / organization targets"]
  Evaluate --> Custom["Custom targets"]
  Builtins --> Result["Boolean flag result"]
  Custom --> Result
  Poll --> Diff["Compare previous and current targets"]
  Diff --> Change["Emit change event"]
Loading

Reviews (2): Last reviewed commit: "fix: Harden evaluation context shape det..." | Re-trigger Greptile

@stanleyphu

Copy link
Copy Markdown
Contributor Author

/devin review

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Starting Devin Review.

Open in Devin Review

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 3 potential issues.

Open in Devin Review

Comment thread src/feature-flags/interfaces/evaluation-context.interface.ts
Comment thread src/feature-flags/evaluator.ts Outdated
Comment thread src/feature-flags/evaluator.ts Outdated
- Classify context shape by value shape, not key presence: unset keys and
  scalar extras on a legacy context no longer trip hybrid rejection
- Normalize the context once per getAllFlags call so an invalid context
  warns once, not once per flag
- Drop the incorrect claim that hybrid contexts fail to compile
users: FlagTarget[];
organizations: FlagTarget[];
/** Absent until the API's custom-targets rollout flag is enabled. */
custom_targets?: FlagCustomTarget[];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 I wonder if it makes sense to roll this out only after custom-targets rollout is enabled to avoid the optional state for custom_targets?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The plan is to have the flag enabled when this is released, but I think we'll still need to keep this optional to avoid things breaking if we have to disable the flag for any reason?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants