Allow deployments to pin the Conductor feature flags - #4205
Merged
Conversation
The Conductor and directory feature flags could only be set per-browser, via /features, so a deployment had no way to configure them for every user. Feature flags can now carry a lockedValue supplied by build-time configuration, which takes precedence over both the persisted user override and the flag's default. Locked flags are read-only on /features and cannot be written back into persisted state. REACT_APP_CONDUCTOR_ENABLE (TRUE/FALSE/unset) pins conductor.enable; while it is TRUE, REACT_APP_LANGUAGE_DIRECTORY_URL, REACT_APP_PLUGIN_DIRECTORY_URL and REACT_APP_MODULES_DIRECTORY_URL pin the corresponding directory URLs. Leaving them unset preserves today's behaviour exactly. The SICP JS override in selectConductorEnable still wins over a pinned flag, since Source is not in the Language Directory yet and a forced-on Conductor would break the textbook's runnable snippets. Also replaces LanguageDirectorySaga's duplicate copy of the default directory URL with the flag's own defaultValue, so pinning the URL to that same public value keeps using the bundled npm copy instead of silently starting a network fetch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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. Comment |
RichDom2185
enabled auto-merge (squash)
July 30, 2026 15:26
RichDom2185
approved these changes
Jul 30, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/new_routes/features.tsx (1)
28-45: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExclude persisted overrides from locked-state styling.
If a user previously modified a flag,
isModifiedremains true after deployment pinning. The pinned Boolean value can appear selected/bold, and the pinned Number value can show a modification asterisk even though neither is user-controlled.Use
!flag.isLocked && modifiedFlag !== undefinedfor both cards.Proposed fix
- const isModified = modifiedFlag !== undefined; + const isModified = !flag.isLocked && modifiedFlag !== undefined;Also applies to: 49-70
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/new_routes/features.tsx` around lines 28 - 45, Update the isModified styling condition in both feature-flag card render paths to require !flag.isLocked alongside modifiedFlag !== undefined. Keep locked flags from rendering selected/bold or modification markers, while preserving existing user-modification styling for unlocked flags.
🧹 Nitpick comments (1)
src/features/directory/flagDirectoryModulesUrl.ts (1)
7-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winComplete coverage for the deployment configuration matrix.
src/features/directory/flagDirectoryModulesUrl.ts#L7-L13: test thatenable: truepins the configured URL, while unset/false preserves normal defaults and overrides.src/commons/sagas/LanguageDirectorySaga.ts#L66-L76: test that the default URL uses bundled languages withoutfetch, while a custom URL loads remote JSON.src/features/conductor/flagConductorEnable.test.ts#L9-L30: addenable: falseand unset cases, especially verifying lockedfalsebeats a user override.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/directory/flagDirectoryModulesUrl.ts` around lines 7 - 13, Complete deployment-matrix test coverage across the affected sites: in src/features/directory/flagDirectoryModulesUrl.ts lines 7-13, verify enabled configuration pins the configured URL while unset or false preserves default behavior and user overrides; in src/commons/sagas/LanguageDirectorySaga.ts lines 66-76, verify the default URL uses bundled languages without fetch and a custom URL fetches remote JSON; in src/features/conductor/flagConductorEnable.test.ts lines 9-30, add unset and false configuration cases, including that locked false overrides user values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 63-64: Update the earlier conductor.enable toggle instructions in
README.md to clarify that users may toggle it back to true only when
REACT_APP_CONDUCTOR_ENABLE is not deployment-pinned; note that setting it to
FALSE makes the control read-only.
- Around line 52-61: Add a language identifier, preferably text, to the fenced
code block containing the directory URL configuration entries in README.md,
without changing its contents.
In `@src/commons/featureFlags/index.ts`:
- Around line 22-24: Update the reducer handling both setFlag and resetFlag so
locked feature flags return without modifying persisted user state. Preserve
existing behavior for unlocked flags, and add a regression test covering
resetFlag on a locked flag.
---
Outside diff comments:
In `@src/new_routes/features.tsx`:
- Around line 28-45: Update the isModified styling condition in both
feature-flag card render paths to require !flag.isLocked alongside modifiedFlag
!== undefined. Keep locked flags from rendering selected/bold or modification
markers, while preserving existing user-modification styling for unlocked flags.
---
Nitpick comments:
In `@src/features/directory/flagDirectoryModulesUrl.ts`:
- Around line 7-13: Complete deployment-matrix test coverage across the affected
sites: in src/features/directory/flagDirectoryModulesUrl.ts lines 7-13, verify
enabled configuration pins the configured URL while unset or false preserves
default behavior and user overrides; in
src/commons/sagas/LanguageDirectorySaga.ts lines 66-76, verify the default URL
uses bundled languages without fetch and a custom URL fetches remote JSON; in
src/features/conductor/flagConductorEnable.test.ts lines 9-30, add unset and
false configuration cases, including that locked false overrides user values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 58ba4496-ed07-432b-8cec-d21485535a2e
📒 Files selected for processing (18)
.env.exampleREADME.mdsrc/commons/featureFlags/FeatureFlag.tssrc/commons/featureFlags/featureSelector.test.tssrc/commons/featureFlags/featureSelector.tssrc/commons/featureFlags/index.test.tssrc/commons/featureFlags/index.tssrc/commons/featureFlags/selectFeatureSaga.test.tssrc/commons/featureFlags/useFeature.test.tsxsrc/commons/featureFlags/useFeature.tssrc/commons/sagas/LanguageDirectorySaga.tssrc/commons/utils/Constants.tssrc/features/conductor/flagConductorEnable.test.tssrc/features/conductor/flagConductorEnable.tssrc/features/directory/flagDirectoryLanguageUrl.tssrc/features/directory/flagDirectoryModulesUrl.tssrc/features/directory/flagDirectoryPluginUrl.tssrc/new_routes/features.tsx
Comment on lines
+63
to
+64
| A deployment can pin these instead of leaving them to each user's browser. Set `REACT_APP_CONDUCTOR_ENABLE` to `TRUE` or `FALSE` in `.env` to fix `conductor.enable` for everyone; while it is `TRUE`, the three `REACT_APP_*_DIRECTORY_URL` variables likewise pin the directory URLs. Pinned flags are shown read-only on the Feature Flags page, and take precedence over anything a user has previously set there. | ||
|
|
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Qualify the earlier toggle instructions for pinned deployments.
When REACT_APP_CONDUCTOR_ENABLE=FALSE, conductor.enable is read-only, so the earlier instruction to toggle it back to true cannot be followed. State that the toggle applies only when the flag is not deployment-pinned.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 63 - 64, Update the earlier conductor.enable toggle
instructions in README.md to clarify that users may toggle it back to true only
when REACT_APP_CONDUCTOR_ENABLE is not deployment-pinned; note that setting it
to FALSE makes the control read-only.
Comment on lines
+22
to
+24
| if (action.payload.featureFlag.isLocked) { | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Guard resets for locked flags too.
setFlag is blocked, but resetFlag can still delete a persisted override for a locked flag. Guard both reducer actions so deployment-pinned flags cannot modify persisted user state, and add a regression test.
Proposed fix
resetFlag<T>(state: FeatureFlagsState, action: { payload: { featureFlag: FeatureFlag<T> } }) {
+ if (action.payload.featureFlag.isLocked) {
+ return;
+ }
delete state.modifiedFlags[action.payload.featureFlag.flagName];
},🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commons/featureFlags/index.ts` around lines 22 - 24, Update the reducer
handling both setFlag and resetFlag so locked feature flags return without
modifying persisted user state. Preserve existing behavior for unlocked flags,
and add a regression test covering resetFlag on a locked flag.
RichDom2185
disabled auto-merge
July 30, 2026 15:28
RichDom2185
force-pushed
the
conductor-deployment-config
branch
from
July 30, 2026 15:32
4e37262 to
324e653
Compare
RichDom2185
approved these changes
Jul 30, 2026
RichDom2185
enabled auto-merge (squash)
July 30, 2026 15:37
RichDom2185
disabled auto-merge
July 30, 2026 15:37
A deployment-pinned flag could still have its persisted override deleted via resetFlag, even though setFlag was already blocked. Guard both reducer actions so a locked flag never modifies persisted user state. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Coverage Report for CI Build 30558636208Coverage increased (+0.1%) to 43.98%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
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
3 participants
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.
Description
Move conductor feature flag and directory flags into env/constants so that it can be set up deployment wide. Also lock the conductor enable feature if the REACT_APP_CONDUCTOR_ENABLE is set to TRUE, and locked to disabled if set to FALSE, and unset allows toggling of the feature flag, like how it currently works (tri-state).
Type of change
How to test
Added some simple tests by Claude, not carefully checked yet, but they do pass.
Checklist