You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(api): block multivariate option changes during live experiments - #8917
The API now rejects multivariate option changes on a feature that is in a running experiment:
MultivariateFeatureOptionViewSet create, update and destroy return 400 while the feature has a running experiment in any environment. Multivariate options belong to the feature, so one experiment locks them everywhere.
Feature create/update: the nested multivariate_options are rejected only when the submitted options differ from the stored ones. The frontend resubmits the whole feature on save, so other fields (description, tags, etc.) still save on a frozen feature.
New helper has_live_experiment in experimentation/feature_state_metadata.py.
Frontend: the create-feature modal already disables the add, edit and remove variant controls when the feature is frozen (useFeatureExperimentFreeze). That check only looks at experiments running in the current environment. Blocking variant edits from the UI for a feature running in another environment needs a project-level experiments query, which is not part of this PR. The API rejects those edits regardless.
How did you test this code?
Backend unit tests:
cd api
pytest tests/unit/features/multivariate/test_unit_multivariate_live_experiment.py
pytest tests/unit/features/ -k "multivariate or mv or feature_serializer or create_feature or update_feature"
The new test module covers create, update and delete of MV options during a live experiment (400), the same create still working without one, and feature PATCH with changed options (400) versus unchanged options plus a metadata change (200). The existing multivariate and feature suites pass.
Multivariate options were only restricted in the frontend. The API
accepted create, update and delete on the MV options endpoint, and
changed nested multivariate_options on feature update, even when the
feature was in a running experiment.
Reject these writes with a 400 while any environment has a running
experiment for the feature. On feature update, only reject when the
submitted options differ from the stored ones, so other feature fields
can still be saved.
ClosesFlagsmith#8766
The API now rejects multivariate option creation, updates, and deletion when the feature has a running experiment in any environment. Feature updates reject changed option content during a running experiment and allow unchanged options. Environment imports raise a validation error before deleting environment-specific state or segments when the feature has a running experiment. The import flow runs in a transaction, and tests cover validation, rollback, and option creation when no experiment is running.
Resolve the import restrictions and resume-locking gap before merging. Valid imports and metadata saves can be rejected, failed imports can appear processed, and concurrent option edits can commit after an experiment resumes.
- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 4 remain after this review.
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 1 remain after this review.
sed -n '1570,1605p' api/uv.lock
python3 - <<'PY'import pathlib, tomllib, urllib.request, io, zipfilelock = tomllib.loads(pathlib.Path('api/uv.lock').read_text())package = next(p for p in lock['package'] if p['name'] == 'flagsmith-common')print('locked version:', package['version'])url = package['wheels'][0]['url']print('wheel:', url)try: with urllib.request.urlopen(url, timeout=20) as r: data = r.read() with zipfile.ZipFile(io.BytesIO(data)) as z: for name in z.namelist(): if name.startswith('task_processor/') and name.endswith('.py') and any(s in name for s in ('decorators', 'task_run_method', 'worker', 'processor')): print('\n---', name, '---') for i, line in enumerate(z.read(name).decode().splitlines(), 1): print(f'{i:4} {line}')except Exception as exc: print(type(exc).__name__, str(exc))PY
Repository: Flagsmith/flagsmith
Length of output: 1957
Script executed:
python3 - <<'PY'import urllib.request, io, zipfile, hashliburl = 'https://files.pythonhosted.org/packages/db/5a/a203df00c5b1cf5fcd6df5857113cf81dcf42b6c6d0887508b86ca808161/flagsmith_common-3.16.1-py3-none-any.whl'try: with urllib.request.urlopen(url, timeout=20) as r: data = r.read() print('wheel sha256:', hashlib.sha256(data).hexdigest()) with zipfile.ZipFile(io.BytesIO(data)) as z: files = [n for n in z.namelist() if n.startswith('task_processor/') and n.endswith('.py')] print('files:', '\n'.join(files)) for name in files: text = z.read(name).decode() if any(s in name for s in ('decorators', 'task_run_method', 'processor', 'worker', 'executor')): print('\n---', name, '---') for i, line in enumerate(text.splitlines(), 1): print(f'{i:4} {line}')except Exception as exc: print(type(exc).__name__, str(exc))PY
When a later feature needs a new option during a running experiment, earlier overwrites remain committed. The import is marked FAILED without a reason, and the UI can report success. Roll back the import on failure and expose the reason.
nl -ba api/tests/unit/features/test_unit_features_views.py | sed -n '2620,2720p'
nl -ba api/experimentation/models.py | sed -n '145,250p'
Repository: Flagsmith/flagsmith
Length of output: 8479
Serialise option changes with experiment starts.
If an experiment starts after validation, changed options still reach the nested update. Replacing options can delete their allocations across environments. Use a shared lock for experiment starts and option writes, then recheck the experiment status before writing.
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 5 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
Minor · Functional Correctness · Quick win
Code analysis · TLA+ model checking
Script executed:
rg -n 'VALID_STATUS_TRANSITIONS|started_at|ExperimentStatus.PAUSED|transition_experiment_status' api/experimentation; sed -n '605,632p' api/experimentation/services.py
Repository: Flagsmith/flagsmith
Length of output: 3106
tla executed:
---- MODULEReviewModel ----
EXTENDSNaturals(* Assumption to verify: resuming a paused experiment retains started_at, so the new feature lock is skipped. Writer phases represent its feature lock, live check, and save. A violation records a save while running.TLCINIT InitNEXT NextINVARIANT Safe*)VARIABLESlive,owner,writer,resumed,violationvars==<<live,owner,writer,resumed,violation>>Init==/\live=FALSE/\owner="none"/\writer="waiting"/\resumed=FALSE/\violation=FALSECheck==/\writer="waiting"/\owner="none"/\owner'="writer"/\writer'=(IFliveTHEN"rejected"ELSE"ready")/\UNCHANGED<<live,resumed,violation>>Resume==/\~resumed/\live'=TRUE/\resumed'=TRUE/\UNCHANGED<<owner,writer,violation>>Save==/\writer="ready"/\writer'="done"/\owner'="none"/\violation'=live/\UNCHANGED<<live,resumed>>Reject==/\writer="rejected"/\writer'="done"/\owner'="none"/\UNCHANGED<<live,resumed,violation>>Idle==/\writer="done"/\resumed/\UNCHANGEDvarsNext==Check\/Resume\/Save\/Reject\/IdleSafe==~violation
====
python3 - <<'PY'import urllib.requesturl='https://raw.githubusercontent.com/postgres/postgres/REL_15_5/src/backend/utils/adt/ri_triggers.c'try: text=urllib.request.urlopen(url, timeout=15).read().decode()except Exception as e: print('FETCH FAILED:',type(e).__name__,str(e))else: lines=text.splitlines() for i,line in enumerate(lines): if line.startswith('RI_FKey_fk_upd_check_required('): print(url) for j in range(i-12, min(i+85,len(lines))): print(f'{j+1}: {lines[j]}') break else: print('Function not found')PY
Repository: Flagsmith/flagsmith
Length of output: 3544
Lock the feature when resuming an experiment.
If an option update passes its check while an experiment is paused, a concurrent resume can commit before the update. The option change can then commit while the experiment is running. Acquire the feature lock for every transition to RUNNING.
If an experiment runs in environment A, a destructive import into environment B fails even when it reuses all existing options. The entire import batch rolls back. Allow this case while still blocking new options and destructive imports into an environment with a running experiment.
Major · Functional Correctness · Heavy lift
Code analysis · TLA+ model checking
Script executed:
rg -n 'overwrite_feature_for_environment|select_for_update|ATOMIC_REQUESTS|isolation_level' api/features/import_export api/experimentation api/app; sed -n '12,84p' api/features/import_export/services.py; sed -n '134,175p' api/features/import_export/tasks.py
Repository: Flagsmith/flagsmith
Length of output: 5261
tla executed:
---- MODULEReviewModel ----
(* The import takes no feature lock. Start abstracts a complete first-start transaction, including its feature lock. It can finish between the import check and write. Violation means the import writes while running.TLCINIT InitNEXT NextINVARIANT Safe*)VARIABLESlive,phase,started,violationvars==<<live,phase,started,violation>>Init==/\live=FALSE/\phase="check"/\started=FALSE/\violation=FALSECheck==/\phase="check"/\phase'=(IFliveTHEN"done"ELSE"write")/\UNCHANGED<<live,started,violation>>Start==/\~started/\started'=TRUE/\live'=TRUE/\UNCHANGED<<phase,violation>>Write==/\phase="write"/\phase'="done"/\violation'=live/\UNCHANGED<<live,started>>Idle==/\phase="done"/\started/\UNCHANGEDvarsNext==Check\/Start\/Write\/IdleSafe==~violation
====
Prevent imports from deleting a newly started rollout.
If an experiment starts after this check, the import can delete its rollout override while the experiment remains running. The experiment then stops serving its configured rollout. Ensure imports cannot remove the override during this transition.
Compare partial options against their stored values.
During a live experiment, a PATCH that resubmits an option’s ID and unchanged string_value can receive HTTP 400 if it omits type or allocation. The comparison treats those fields as None and blocks the description save. Resolve omitted fields from the identified option in both checks.
The reason will be displayed to describe this comment to others. Learn more.
Trivial · Maintainability & Code Quality · Low value
Supported by static analysis
Script executed:
sed -n '40,165p' api/tests/unit/features/import_export/test_unit_features_import_export_tasks.py
sed -n '1,115p' api/features/import_export/services.py
sed -n '125,185p' api/features/import_export/tasks.py
Repository: Flagsmith/flagsmith
Length of output: 9584
Script executed:
printf'%s\n''--- import task test module ---'
nl -ba api/tests/unit/features/import_export/test_unit_features_import_export_tasks.py | sed -n '1,165p'printf'%s\n''--- test coverage in import/export tests ---'
rg -n -i 'rollback|rolls.back|atomic|FAILED|overwrite|enabled' api/tests/unit/features/import_export
printf'%s\n''--- feature fixture ---'
nl -ba api/tests/conftest.py | sed -n '625,660p'printf'%s\n''--- model state creation ---'
nl -ba api/features/models.py | sed -n '115,135p;175,200p;915,950p'
Repository: Flagsmith/flagsmith
Length of output: 20115
Use a different initial enabled value.
The initial and imported values are both False, so the assertion cannot detect an imported False replacing the initial value. Start with True to test preservation after rejection.
- assert feature.feature_states.get(environment=environment).enabled is False+ assert feature.feature_states.get(environment=environment).enabled is True
This branch has not been deployed
No deployments
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
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Contributes to #8766
The API now rejects multivariate option changes on a feature that is in a running experiment:
MultivariateFeatureOptionViewSetcreate, update and destroy return 400 while the feature has a running experiment in any environment. Multivariate options belong to the feature, so one experiment locks them everywhere.multivariate_optionsare rejected only when the submitted options differ from the stored ones. The frontend resubmits the whole feature on save, so other fields (description, tags, etc.) still save on a frozen feature.has_live_experimentinexperimentation/feature_state_metadata.py.Frontend: the create-feature modal already disables the add, edit and remove variant controls when the feature is frozen (
useFeatureExperimentFreeze). That check only looks at experiments running in the current environment. Blocking variant edits from the UI for a feature running in another environment needs a project-level experiments query, which is not part of this PR. The API rejects those edits regardless.How did you test this code?
Backend unit tests:
The new test module covers create, update and delete of MV options during a live experiment (400), the same create still working without one, and feature PATCH with changed options (400) versus unchanged options plus a metadata change (200). The existing multivariate and feature suites pass.