Validate reserved state names before registration - #7136
Merged
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Merging this PR will not alter performance
Comparing 
Footnotes
|
FarhanAliRaza
marked this pull request as draft
September 14, 2026 17:42
FarhanAliRaza
marked this pull request as ready for review
September 17, 2026 17:19
Merged
6 tasks
…ten the validating metaclass Set the deprecation version to the next release, fold the colliding name into the deprecation key so each collision in a class is reported, move the inherited-member check into a helper, and drop the unneeded override marker on BaseState.add_field. The shadowed-touched-method tests from reflex-dev#7132 opt into the legacy flag.
Reserved names are always rejected. The tests that constructed states with a reserved backend var can no longer exist and are removed; the fast-path test keeps its marked-override case.
Reserved-name validation means only a marked method override can define a fast-pathed framework name, so the recursive prune after dynamic registration is removed.
masenf
approved these changes
Sep 17, 2026
masenf
pushed a commit
that referenced
this pull request
Sep 17, 2026
#7136 landed, so the bespoke `_check_reserved_router_names` can go. It is not #7136's validator that covers these names -- that one pops state vars out of its reserved set, so the `rx_router_*` fields are not in it. What rejects them is the framework's general inherited-var shadow detection: a substate redeclaring `rx_router_session` shadows a `BaseState` var like any other field and raises BaseVarShadowsInheritedVarError. That is a better mechanism than a router-specific list, so the check is redundant. Verified before removing: an annotated field, an unannotated class attribute and a computed var are all rejected on a substate, and `router` itself is rejected by #7136's validator. Two cases are not covered, both general rather than router-specific, so neither is papered over here: - a mixin declaring an inherited var is not rejected, and neither is a state consuming it; the field is silently shadowed. This is not about the router -- a mixin can shadow any inherited base var, while a direct subclass doing the same is correctly rejected. - a direct `BaseState` subclass starts its own root, so there is no inherited var to shadow. `test_router_field_names_are_reserved` is narrowed to the substate path it actually guarantees, and the breaking-change fragment now names the error callers will really see. The only merge conflict was the `reflex.istate.data` import list in tests/units/test_state.py; resolved as the union of both sides. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh
masenf
pushed a commit
that referenced
this pull request
Sep 19, 2026
…o, probe notes, tools FINDING-001: rx.State's new _StateMeta (#7136) makes any metaclass derived from reflex.vars.BaseStateMeta unusable on a State subclass; reflex-enterprise 0.9.5 OIDC auth fails to import on 0.9.12a1 and imports on 0.9.11.post1. Framework-only repro included. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NeyxWsuC9hqSKq8YcEwZyY
masenf
pushed a commit
that referenced
this pull request
Sep 19, 2026
#7136 added `_StateMeta(BaseStateMeta)` and made `class BaseState(EvenMoreBasicBaseState, metaclass=_StateMeta)`, so `type(rx.State)` stopped being `reflex_base.vars.BaseStateMeta`. A downstream metaclass written as `class M(BaseStateMeta)` is then a sibling of `_StateMeta` rather than a subclass, and `class S(rx.State, metaclass=M)` fails with `TypeError: metaclass conflict`. reflex-enterprise 0.9.5 does exactly that in `reflex_enterprise/auth/oidc/state.py`, so every AuthPlugin / MCPPlugin / EventHandlerAPIPlugin app dies at startup. The conflict is raised by `__build_class__` while it resolves the most derived metaclass, before any metaclass code runs, so nothing on `_StateMeta` can intercept it: `type(rx.State)` has to be `BaseStateMeta` again. The reserved-name validation therefore moves into `BaseStateMeta.__new__`, as a hook `reflex.state` installs once `BaseState` exists, and `BaseState` goes back to inheriting `BaseStateMeta`. The check is guarded on "a base is a BaseState", so plain models built on the same metaclass keep their own namespace. Fixes #7211 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NeyxWsuC9hqSKq8YcEwZyY (cherry picked from commit 6ab3edbc941c0bc3cfc2e2443a65347c6af03045)
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
2 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.
State vars named
_get_was_touchedor_update_was_touchedcan replace framework helpers and crash persistence or cleanup. Other collisions, includingget_fieldsanddirty_vars, can break class creation or bookkeeping. This rejects reserved declarations before field collection and state initialization, and applies the same validation to dynamic vars, fields, event handlers, and route arguments.Reserved names are derived once from
BaseStateand its framework bases. Ordinary vars, inherited user vars, Python protocols, and explicitly marked method overrides remain supported. Validation also checks state mixins, ordinary Python mixins, and inherited model fields without invoking descriptors.This changes the default behavior. Existing apps can temporarily set
REFLEX_STATE_ALLOW_RESERVED_NAMES=1to retain legacy handling of conflicting vars with a deprecation warning until 1.0. The flag preserves the old behavior, including collision-related crashes; renaming conflicting declarations resolves them. Migration guidance and fragments for both affected packages are included.Supersedes #7132 and the method-specific follow-up fix for #7135.
Closes #7091.
Closes #7135.
Validation:
All 35 new cases pass; on the unchanged base, the 33 regression cases fail and the two compatibility controls pass.
The final state suites pass all 312 tests.
Full unit suite: 8,626 passed, 18 skipped. The coverage gate exits nonzero at 71.34%, below the required 72%; the unchanged base also reports 71.34% with 8,591 passed and 18 skipped. The new validator has 97.59% coverage; state plus validation have 86.19%.
Full
pyright reflex testspasses. All commit hooks pass, including Ruff, codespell, stub generation, Pyright, and ty.Repository Ruff checks and formatting pass with the ignored
ignore/scratch tree excluded.Followed CONTRIBUTING.md and added regression tests, migration documentation, and package news fragments.
Breaking change with a temporary compatibility opt-in.
Checked overlapping work: this is the centralized replacement for Call _get_was_touched through BaseState so a state var cannot shadow it #7132 and the narrow A backend var named _update_was_touched breaks state cleanup and persistence checks #7135 follow-up.