fix: deliver linked state updates to all clients - #6934
fix: deliver linked state updates to all clients#6934benedikt-bartscher wants to merge 9 commits into
Conversation
Greptile SummaryThis PR extends shared-state fan-out to clients connected through other backend instances and strengthens Redis token ownership reconciliation.
Confidence Score: 4/5The PR is not yet safe to merge because an ownership transition between the connectivity check and update emission can still cause a linked client to miss its state update. The shared-state task checks connectivity before entering the state-update path, while emission later resolves routing independently; a client that moves backend instances in that interval can still have the delta sent using stale ownership. Files Needing Attention: reflex/istate/shared.py
|
| Filename | Overview |
|---|---|
| reflex/istate/shared.py | Cross-instance fan-out now checks distributed connectivity, but the check remains separate from eventual update routing, leaving the previously reported ownership-change race outstanding. |
| reflex/utils/token_manager.py | Adds distributed connectivity checks and conditional ownership recovery while addressing the previously reported stale-cache and delayed-deletion paths. |
| tests/units/istate/test_shared.py | Adds focused coverage for local and cross-instance shared-state fan-out. |
| tests/units/utils/test_token_manager.py | Adds regression coverage for disconnect ordering, stale ownership records, delayed notifications, and conditional restoration. |
Reviews (8): Last reviewed commit: "wip" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Merging this PR will not alter performance
Comparing 
Footnotes
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| async def _update_client(token: str): | ||
| # Don't send updates for disconnected clients; emit_update relays the | ||
| # delta to the owning instance if the socket lives elsewhere. | ||
| if not await token_manager.is_token_connected(token): |
There was a problem hiding this comment.
If a linked client disconnects from one backend and reconnects to another after this connectivity check but before modify_state emits the delta, emit_lost_and_found reuses the cached former owner without refreshing it, causing the update to be published to the old backend and missed by the reconnected client.
Knowledge Base Used: State management and event processing
There was a problem hiding this comment.
The check itself refreshes the record from Redis, so staleness can only develop while modify_state runs. If the owner moves in that window, the publish lands at the former owner, which no longer has a local mapping (disconnect drops it before any Redis I/O), re-resolves the owner with a fresh Redis fetch, and forwards the update — delivered with one extra hop. If the former owner died instead, its Redis record survives, so the reconnecting client receives a new token via duplicate detection and the old token is abandoned. Refreshing before publish would add a GET per relayed update and still be TOCTOU; the one-hop forwarding is what closes this gap.
There was a problem hiding this comment.
You're right. I missed the forwarding behavior in emit_update: the connectivity check only establishes a best-effort target. If ownership changes during modify_state, the stale target receives the message, sees no local mapping because disconnect removes it before Redis I/O, refreshes the owner, and forwards the update to the new instance. If the former instance has died, reconnect duplicate detection assigns a new token, so the old token has no live client. Refreshing before publish would add a Redis GET while remaining inherently TOCTOU. I'll withdraw this finding; no code change is needed.
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Disconnects pop the local mapping before touching redis, so a self-owned record still present when a del notification arrives was created by a newer link and refers to a live socket. Re-store it instead of dropping it, which previously broke linked-update delivery for clients that relinked before the notification was processed.
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…claims The keep-alive restore in _handle_socket_record_del is only legitimate when the key is genuinely gone. A plain SET could overwrite a record written meanwhile by another instance claiming the expired token, or by a concurrent relink on this instance. With NX, any newer write wins, and a wrongful delete of an owned key self-heals via the del notification.
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
1 participant
Uh oh!
There was an error while loading. Please reload this page.