Repository navigation
Isolate local and remote flag keys - #311
Merged
Merged
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved duplicate accessors and an outdated test stub break generated-code compilation, while remote fake values and long Java Preferences keys also regress.
5 open issues
What changed in this PR
This PR addresses #294 by separating same-name local and remote flags throughout Featured’s runtime, storage providers, and generated declarations.
Changes:
- Adds
FlagScopeand scope-qualified storage/cache keys while preservingANYresolution for existing constructor and factory calls. - Updates generators and aggregation to emit explicit scopes and permit duplicate public names.
- Adds isolation tests, updates runtime fixtures, and documents the storage change without migrating existing bare keys.
| File | Description |
|---|---|
providers/sharedpreferences/src/test/kotlin/dev/androidbroadcast/featured/sharedpreferences/SharedPreferencesProviderConfigTest.kt |
Tests scoped storage, removal, and remote resolution. |
providers/sharedpreferences/src/main/kotlin/dev/androidbroadcast/featured/sharedpreferences/SharedPreferencesProviderConfig.kt |
Uses scoped keys for storage and notifications. |
providers/nsuserdefaults/src/iosTest/kotlin/dev/androidbroadcast/featured/nsuserdefaults/NSUserDefaultsConfigValueProviderTest.kt |
Tests scoped direct writes and ignored bare keys. |
providers/nsuserdefaults/src/iosMain/kotlin/dev/androidbroadcast/featured/nsuserdefaults/NSUserDefaultsConfigValueProvider.kt |
Uses scoped keys for persistence and observation. |
providers/javaprefs/src/main/kotlin/dev/androidbroadcast/featured/javaprefs/JavaPreferencesConfigValueProvider.kt |
Uses scoped preference keys. |
providers/datastore/src/jvmTest/kotlin/dev/androidbroadcast/featured/datastore/DataStoreConfigValueProviderFlowTest.kt |
Tests cross-scope observation isolation. |
providers/datastore/src/commonMain/kotlin/dev/androidbroadcast/featured/datastore/DataStoreConfigValueProvider.kt |
Scopes storage keys and deduplicates observed values. |
featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/ConfigParamGeneratorTest.kt |
Tests generated scopes and duplicate declarations. |
featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGeneratorTest.kt |
Tests scoped registry entries and retained duplicates. |
featured-gradle-plugin/src/test/kotlin/dev/androidbroadcast/featured/gradle/aggregation/FeaturedAggregationDuplicateKeyTest.kt |
Changes duplicate-key expectations to acceptance. |
featured-gradle-plugin/src/test/fixtures/wiring-kotlin-jvm-project/src/main/kotlin/dev/androidbroadcast/featured/FeaturedRuntimeStub.kt |
Adds scoped parameters to the JVM wiring fixture. |
featured-gradle-plugin/src/test/fixtures/wiring-kmp-project/src/commonMain/kotlin/dev/androidbroadcast/featured/FeaturedRuntimeStub.kt |
Adds scoped parameters to the multiplatform wiring fixture. |
featured-gradle-plugin/src/test/fixtures/wiring-android-library-project/src/main/kotlin/dev/androidbroadcast/featured/FeaturedRuntimeStub.kt |
Updates the Android library wiring stub. |
featured-gradle-plugin/src/test/fixtures/wiring-android-app-project/src/main/kotlin/dev/androidbroadcast/featured/FeaturedRuntimeStub.kt |
Updates the Android app wiring stub. |
featured-gradle-plugin/src/test/fixtures/kmp-publish-project/module/src/commonMain/kotlin/dev/androidbroadcast/featured/FeaturedRuntimeStub.kt |
Updates the multiplatform publication stub. |
featured-gradle-plugin/src/test/fixtures/android-project/src/main/kotlin/dev/androidbroadcast/featured/FeaturedRuntimeStub.kt |
Updates the Android project stub. |
featured-gradle-plugin/src/test/fixtures/android-library-project/src/main/kotlin/dev/androidbroadcast/featured/FeaturedRuntimeStub.kt |
Updates the Android library stub. |
featured-gradle-plugin/src/test/fixtures/android-dce-project/src/main/kotlin/dev/androidbroadcast/featured/FeaturedRuntimeStub.kt |
Updates the dead-code-elimination fixture stub. |
featured-gradle-plugin/src/test/fixtures/aggregator-multi-module-project/app/src/main/kotlin/dev/androidbroadcast/featured/FeaturedRuntimeStub.kt |
Updates the multi-module aggregation stub. |
featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/ConfigParamGenerator.kt |
Emits scopes and deduplicates parameter declarations. |
featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GenerateFeaturedRegistryTask.kt |
Removes duplicate-key rejection. |
featured-gradle-plugin/src/main/kotlin/dev/androidbroadcast/featured/gradle/aggregation/GeneratedFeaturedRegistryGenerator.kt |
Emits registry scopes from manifest kinds. |
docs/specs/isolate-local-remote-keys.md |
Defines isolation behavior and compatibility boundaries. |
core/src/jvmTest/kotlin/dev/androidbroadcast/featured/ConfigParamConstructorSignatureTest.kt |
Checks the retained seven-parameter constructor. |
core/src/commonTest/kotlin/dev/androidbroadcast/featured/InMemoryConfigValueProviderTest.kt |
Tests cross-scope notification isolation. |
core/src/commonTest/kotlin/dev/androidbroadcast/featured/ConfigValuesScopeIsolationTest.kt |
Tests scoped resolution, snapshots, and provider routing. |
core/src/commonTest/kotlin/dev/androidbroadcast/featured/ConfigParamTest.kt |
Tests scopes, storage identities, and equality. |
core/src/commonMain/kotlin/dev/androidbroadcast/featured/ValueResolutionStrategy.kt |
Documents scope-filtered resolution candidates. |
core/src/commonMain/kotlin/dev/androidbroadcast/featured/InMemoryConfigValueProvider.kt |
Scopes in-memory values and change signals. |
core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt |
Routes reads by scope and isolates cached values. |
core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigParam.kt |
Adds scopes, storage identities, and compatibility overloads. |
CHANGELOG.md |
Announces isolation and the storage-key change. |
AGENTS.md |
Updates runtime and flag-declaration guidance. |
Review effort: Balanced
Comment on lines
+105
to
+106
| // Same key in one object is last-write-wins so the file compiles. Cross-scope names | ||
| // live in different objects and are both emitted. |
| if (entries.isEmpty()) return "" | ||
| // Same key in one object is last-write-wins so the file compiles. Cross-scope names | ||
| // live in different objects and are both emitted. | ||
| val emitted = entries.associateBy { it.key }.values |
| buildList { | ||
| add("key = \"$key\"") | ||
| add("defaultValue = ${formatDefault()}") | ||
| add("scope = $scope") |
Comment on lines
+287
to
+288
| FlagScope.REMOTE -> null | ||
| FlagScope.LOCAL, FlagScope.ANY -> knownLocal ?: readLocal(param) |
| writers[klass] | ||
| ?: throw IllegalArgumentException("Unsupported type: $klass. Register a TypeConverter for custom types.") | ||
| writer(node, param.key, value) | ||
| writer(node, param.localStorageKey(), value) |
Record the FlagScope contract so the same name in localFlags and remoteFlags is no longer one store, without a major API break.
Key the snapshot and local storage by FlagScope plus the public key so FEATURE_X in localFlags and remoteFlags are different slots. The old constructor stays ANY, so already compiled callers keep local, then remote, then default. Remote providers still look up the public key. There is no migration of a previously stored bare key.
kirich1409
force-pushed
the
fea-18-isolate-local-remote
branch
from
November 6, 2026 22:48
b731fb1 to
083a659
Compare
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
The develop stripping test copies jvm-dce-project and compiles generated flags. That stub still had only the pre-scope factory, so FlagScope.LOCAL did not resolve.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
This was referenced Nov 6, 2026
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.
Closes #294
Type of change
Checklist
./gradlew spotlessCheckpassesNotes for reviewers
A local
FEATURE_Xand a remoteFEATURE_Xare no longer one store.ConfigParamgainsFlagScope. Generated local flags areLOCAL, generated remote flags areREMOTE, and the old constructor and factory stayANY, so already compiled callers keeplocal ?: remote ?: default. This is not a major-version API break.Snapshot and local storage use
LOCAL:FEATURE_X,REMOTE:FEATURE_X, andANY:FEATURE_X. Remote providers, including Firebase and ConfigCat, still look up the public key. A stored override of a remote flag is not a resolution candidate: underValueResolutionStrategy.Default,getValueof that flag staysremote ?: default. There is no migration of a previously stored bare key. Duplicate names are not rejected. Same scope and key stay last-write-wins.The debug screen still keys rows by the public key. A same-name pair can still share one debug row.
Spec:
docs/specs/isolate-local-remote-keys.md.Accepted on
0a3ef15, not the earlierb731fb1tree. That commit only updates the JVM DCE fixture so it keeps the oldConfigParamfactory and addsFlagScopeplus the scoped overload.JvmFlagStrippingTestpassed locally after that fix.Local acceptance used JDK 17 and
ANDROID_HOME::core:jvmTest,:providers:datastore:jvmTest,:providers:sharedpreferences:testDebugUnitTest,:providers:javaprefs:test,:featured-gradle-plugin:test(355 tests, 0 failures, 0 skipped, including Android fixtures andJvmFlagStrippingTest),:featured-shrinker-tests:test,:core:koverVerify,spotlessCheck, and:providers:nsuserdefaults:iosSimulatorArm64Test(including the storage-identity and bare-key cases). No device screenshots; the debug screen was not changed.