ENG-9350: App.modify_state can directly modify SharedState tokens#6336
ENG-9350: App.modify_state can directly modify SharedState tokens#6336
Conversation
When a SharedState is modified directly by its shared token (e.g. from an API route webhook), propagate dirty vars to all clients linked to that shared state — matching the behavior that already exists when modifying shared state through a private client token. - Add _collect_shared_token_updates on SharedStateBaseInternal to detect the shared-token case inside _modify_linked_states and propagate to linked clients via the existing _do_update_other_tokens mechanism - Add App.set_contexts / App._set_contexts_internal to centralize pushing RegistrationContext and EventContext into the current contextvars scope, replacing the old _registration_context_middleware - App.modify_state now calls set_contexts so out-of-band callers (API routes, webhooks) get the necessary contexts automatically - Integration test: API endpoint modifies two SharedState subclasses by shared token, asserts both propagate to two linked browser tabs, and verifies normal event handlers still work afterward - Unit tests for set_contexts covering all combinations of pre-existing / absent contexts, no-event-processor, and reset-on-exit behavior Closes #6335
Greptile SummaryThis PR enables The implementation adds Confidence Score: 5/5Safe to merge; the only finding is a minor consistency nit with no impact on current behaviour. The core logic — detecting the shared-token path via empty _reflex_internal_links, delegating to _collect_shared_token_updates, and injecting contexts via set_contexts — is correct. The truthiness-vs-is-not-None difference in the new helper is a style inconsistency only; _previous_dirty_vars is always non-empty when vars are genuinely dirty. Integration and unit tests cover multi-SharedState propagation, context setup/teardown for all four pre-existing-context combinations, and verified post-API normal event handling. reflex/istate/shared.py — the _previous_dirty_vars truthiness check (P2 style nit). Important Files Changed
Sequence DiagramsequenceDiagram
participant API as API Route / Webhook
participant App as App.modify_state
participant SC as set_contexts()
participant SM as StateManager
participant SBI as SharedStateBaseInternal
participant CS as _collect_shared_token_updates
participant DU as _do_update_other_tokens
participant C1 as Client 1 (private token)
participant C2 as Client 2 (private token)
API->>App: modify_state(BaseStateToken(shared_token, SharedState))
App->>SC: set_contexts() → push RegistrationContext + EventContext
App->>SM: modify_state_with_links(shared_token)
SM->>SBI: _modify_linked_states()
Note over SBI: _reflex_internal_links is empty (shared token case), _held_locks remains {}
SBI->>SBI: yield (caller modifies ss.counter, notes.note)
Note over SBI: _held_locks_linked_states() returns [] → normal loop is no-op
SBI->>CS: _collect_shared_token_updates(affected_tokens, current_dirty_vars)
CS->>CS: iterate substates, add _linked_from clients to affected_tokens, populate current_dirty_vars
CS-->>SBI: affected_tokens = {client1, client2}, current_dirty_vars = {...}
SBI->>DU: _do_update_other_tokens(affected_tokens, current_dirty_vars)
DU->>C1: emit_update (delta for counter + note)
DU->>C2: emit_update (delta for counter + note)
App->>SC: reset contexts on exit
Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
| if substate._previous_dirty_vars: | ||
| current_dirty_vars[substate.get_full_name()] = set( | ||
| substate._previous_dirty_vars | ||
| ) | ||
| if substate._get_was_touched() or substate._previous_dirty_vars: | ||
| affected_tokens.update(substate._linked_from) |
There was a problem hiding this comment.
Truthiness check diverges from the existing
is not None pattern
The existing collection loop (lines 376–383) uses is not None to decide whether to populate current_dirty_vars and affected_tokens, treating an empty set() the same as a populated one. This new method uses a truthiness check, so an empty _previous_dirty_vars = set() (the field's initialiser value) skips the current_dirty_vars entry even when _get_was_touched() is True.
In practice _previous_dirty_vars will be non-empty whenever vars were actually modified, so this is unlikely to surface as a bug; but aligning with the is not None guard used in the sibling loop keeps the two paths consistent and easier to reason about.
| if substate._previous_dirty_vars: | |
| current_dirty_vars[substate.get_full_name()] = set( | |
| substate._previous_dirty_vars | |
| ) | |
| if substate._get_was_touched() or substate._previous_dirty_vars: | |
| affected_tokens.update(substate._linked_from) | |
| if substate._previous_dirty_vars is not None: | |
| current_dirty_vars[substate.get_full_name()] = set( | |
| substate._previous_dirty_vars | |
| ) | |
| if substate._get_was_touched() or substate._previous_dirty_vars is not None: | |
| affected_tokens.update(substate._linked_from) |
When a SharedState is modified directly by its shared token (e.g. from an API route webhook), propagate dirty vars to all clients linked to that shared state — matching the behavior that already exists when modifying shared state through a private client token.
Closes #6335