Repository navigation
fix(core): isolate malformed FFE flags - #363
Closed
leoromanovsky wants to merge 1 commit into
Closed
leoromanovsky wants to merge 1 commit into
leoromanovsky wants to merge 1 commit into
Conversation
Implement strict SemVer comparison and isolate malformed flag definitions with PARSE_ERROR while preserving valid neighboring flags. Environment: Datadog workspace
leoromanovsky
marked this pull request as ready for review
August 11, 2026 17:02
leoromanovsky
requested review from
btthomas and
pavlokhrebto
and removed request for
a team
August 11, 2026 17:02
There was a problem hiding this comment.
Pull request overview
Adds per-flag validation and SemVer operators while isolating malformed flags.
Changes:
- Implements strict SemVer parsing and comparison.
- Validates flag structures and returns
PARSE_ERROR. - Updates canonical fixtures and error-code assertions.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
packages/core/src/evaluation/evaluateForSubject.ts |
Adds per-flag validation and parse errors. |
packages/core/src/evaluation/rules.ts |
Validates rules and evaluates SemVer operators. |
packages/core/src/evaluation/semver.ts |
Implements SemVer parsing and precedence. |
packages/core/test/TestCaseResult.types.ts |
Supports expected error codes. |
packages/core/test/evaluation/flags-v1.spec.ts |
Asserts fixture error codes. |
packages/core/test/ffe-system-test-data |
Advances canonical fixture revision. |
packages/node-server/index.d.ts |
Exposes SemVer condition declarations. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| logger.debug(`returning default assignment because flag is disabled`, { | ||
| if (!isValidFlag(flag)) { | ||
| logger.debug(`returning default assignment because flag configuration is invalid`, { | ||
| flagKey: flag.key, |
Comment on lines
+164
to
+166
| if (!isVariantType(flag.variationType) || !isRecord(flag.variations) || !Array.isArray(flag.allocations)) { | ||
| return false | ||
| } |
| (split) => | ||
| isRecord(split) && | ||
| typeof split.variationKey === 'string' && | ||
| split.variationKey in variations && |
leoromanovsky
marked this pull request as draft
August 11, 2026 17:10
4 tasks done
Contributor
|
Do we still need this? |
Collaborator
Author
|
🤖 Comment from Codex working with @leoromanovsky: In reply to your question: no, this PR is superseded by #370, which explicitly took it over and merged on August 24. That merge includes the malformed-flag validation, canonical fixture error-code assertions, and fixes for the three bot findings here. It also extends SemVer support beyond this PR's original implementation. I recommend closing #363 as superseded. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
The canonical FFE fixtures added through DataDog/ffe-system-test-data#26 require malformed flags to return
ERROR/PARSE_ERRORwithout poisoning valid neighboring flags. The earlier Dependabot-only bump in #362 also exposed missing SemVer comparison support and is red in CI.Changes
ffe-system-test-datasubmodule toea8b5cc5ce335109f11f3efbc5fd608f98a3ca54.ERRORwithPARSE_ERRORfor malformed flags while keeping the rest of the configuration usable.Decisions
Validation:
yarn lintyarn format:checkyarn workspace @datadog/flagging-core typecheckyarn buildyarn test(518 tests)Supersedes #362.