Clear sync state on user logout#2707
Conversation
Dispatch a shared userLoggedOut action on logout and invalid token detection that resets sync slices, cancels active operations, stops pollers, and clears authenticated RTK Query caches.
katinthehatsite
left a comment
katinthehatsite
left a comment
There was a problem hiding this comment.
Thanks for making the changes @gcsecsey - nice work!
I found some issues on my end when testing this PR. This is what I saw on my end: I started the pull operation and waited until the downloading backup step. Then, I logged out and noticed that some of the buttons were stuck in the "Importing" state:
(see the top right corner)
Then, when I log in as a different user, I see the following issue on the import screen:
When importSite fails during a sync pull, the pull Redux state stays at 'importing' while the import context fires an error event. Since canCancelPull only allows cancellation for 'in-progress' and 'downloading' states, the user gets stuck with a disabled dismiss button. Add an isError flag to ImportProgressState and set it on import errors. In the sync UI, detect when a pull is in 'importing' state but the import has failed, and treat it as a pull error so the user can dismiss it and retry.
Thanks for the review @katinthehatsite, great catch! This was caused by the import/export also being tracked in the ImportExportProvider context, not just Redux. I set this to also be reset on logout. I also encountered a stuck state when trying to pull a site that caused a db import issue: 
I updated the component to treat this state as a pull error as well, to unblock this state. However, I'm not sure if this is the best approach to handle this. I think ideally we'd show the pull operation as completed, and separately also show the db import error in a dialog, similarly to how it's done when importing, right? |
📊 Performance Test ResultsComparing a73a48e vs trunk app-size
site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff) |
katinthehatsite
left a comment
katinthehatsite
left a comment
There was a problem hiding this comment.
Thanks for making the changes @gcsecsey ! I was testing the flow a bit more but now I am getting strange session expired messages:
If I do manage to log in, I am getting the errors related to not being logged in while doing the push or pull:
I did not debug the code for this but wanted to point this out early in the review.
Thanks for testing again @katinthehatsite! I could reproduce this issue, and found that it was caused by the wpcomClient being cleared after the logout event being dispatched, causing refetches to run into the error. I also found that even though I added the listener earlier for clearing the state, these weren't firing because the listener iterated on the updated (empty) state. I fixed both of these issues now and the whole pull flow seems to be working well on my end: CleanShot.2026-03-19.at.13.54.07.mp4I'm still working to fix an issue with cancelling pushes, then I'll ask for another review. |
…tomattic/studio into gcsecsey/stu-166-clear-sync-on-logout
…logout Extracted PUSH_POLLERS/PULL_POLLERS maps and stop functions into a shared sync-pollers module. Cancel thunks now call stopPushPoller/stopPullPoller directly, so in-flight poll requests see signal.aborted and exit silently instead of showing error modals.
|
@katinthehatsite @fredrikekelund I think this is ready for another review. I fixed these issues for the push flow; I reverted emptying the connected sites cache on logout to avoid resetting the connected site list when logging back in: CleanShot.2026-03-19.at.14.02.16.mp4I also updated the pollers to avoid receiving errors when logging out at a later step in the push process, when the changes are being processed on the server: CleanShot.2026-03-19.at.14.40.35.mp4 |
fredrikekelund
left a comment
fredrikekelund
left a comment
There was a problem hiding this comment.
I took another look at the Linear issue, and it doesn't appear like these changes fully address the issue.
If I have two users with access to the same WP.com site, initiate a push to the site with my first account, log out in Studio, then log in with my second account (which has also connected the same WP.com site to my local site), then I still see Push cancelled in the interface. One step better than seeing an error message, but the UI should really be completely cleared.
katinthehatsite
left a comment
katinthehatsite
left a comment
There was a problem hiding this comment.
I tried testing the changes again but I am still seeing the error with pushing a site when I log in and try to do the push operation:
I tried with a couple of different users but I keep on running into the same error.
katinthehatsite
left a comment
katinthehatsite
left a comment
There was a problem hiding this comment.
Log in again (same or different account) and check that initial pull/push states are shown
I am re-testing this again and when I log back in, I am seeing Push cancelled even though it was in progress when I logged out:
Just to confirm, is this the expected behaviour?
katinthehatsite
left a comment
katinthehatsite
left a comment
There was a problem hiding this comment.
And another sidenote is that file it shows Push cancelled on the Sync tab, I am seeing in Import/Export tab that the sync is in progress:
Good catch, sorry for missing this earlier. No, I think it'd be better behavior to completely clear the state in this case, and not show the success message.
Thanks for catching, this is also clearly a bug. I'll also address this one. |
|
I fixed both issues, now the state is completely reset on logout: CleanShot.2026-04-15.at.17.22.46.mp4 |
|
@katinthehatsite @fredrikekelund could you please give this another look when you have some time? Thanks! 🙏 |
|
@gcsecsey, yep, I'll take another look later today. Sorry for the delay |
fredrikekelund
left a comment
fredrikekelund
left a comment
There was a problem hiding this comment.
This issue is really surprisingly complex… 🤔 I raised a point about how we handle active import operations when a user logs out. I think it's OK if this PR doesn't have a perfect solution to that problem, but we should at least be explicit in the code and acknowledge that we don't/can't abort those operations.
Also, I'd like to clarify how the sync operation state gets updated when displayNotification=true in the cancel{Push,Pull}Thunk functions.
…tomattic/studio into gcsecsey/stu-166-clear-sync-on-logout
Replace per-operation cancel thunk dispatches with a single abortAllSyncOperations() helper. The slice's addCase(userLoggedOut) already resets pullStates/pushStates, so the listener only needs to abort in-flight side effects (pollers, push upload aborts, main-process operations) — no state dispatches required. This removes the displayNotification param from cancelPush/cancelPull, which was overloaded to gate both the toast and the post-logout state re-population. The thunks are now purely user-initiated cancel paths.
An active pull's import phase is driven by main-process events that keep emitting after logout, so resetting importState here was futile — it would just get repopulated. Snapshot the pre-logout pull-active flag via a ref (pullStates is reset synchronously on userLoggedOut, so the post-logout selector value is already false), and only clear when no pull was active. The hook now reads from the Redux store, so the test wrapper needs a Provider too.
Conflicts in apps/studio/src/hooks/use-import-export.tsx and the deletion of its test file: trunk's #3129 refactored the hook to depend on the CLI for import/export, removing Sentry, useSiteDetails, error formatting, and the BackupArchiveInfo path. Took trunk's simpler structure and re-applied the logout cleanup block (useAuth + useRootSelector + useRef-based pre-logout pull-active snapshot driving the setImportState/setExportState reset). Dropped the isError flag on ImportProgressState and the matching pullImportFailed workaround in sync-connected-sites.tsx: trunk's pull thunk now propagates importSite errors via pullStates.failed, so the "dismiss button stuck disabled" bug those addressed is no longer reachable. Accepted trunk's deletion of use-import-export.test.tsx as part of the legacy-test cleanup.
fredrikekelund
left a comment
fredrikekelund
left a comment
There was a problem hiding this comment.
This is looking pretty good, but there's an issue where the sidebar spinner doesn't disappear after logging out
|
@fredrikekelund I rebased the PR and followed up on your comments, except for the issue where the sidebar spinner doesn't disappear after logging out. I'll look into that tomorrow and ping you again when it's ready for another review. |
|
@fredrikekelund @katinthehatsite I think this is ready for another review. While working on fixing the spinner state, I found that it's a symptom of not having the same abort callbacks for pulls as for pushes. I updated the behavior so that logging out during the download phase will stop without the error dialog from before, and will clear the spinner too. During the import phase, the import is local, so we let it finish, keeping the spinner visible until it completes. I think that’s the right tradeoff, but happy to revisit. WDYT? |
fredrikekelund
left a comment
fredrikekelund
left a comment
There was a problem hiding this comment.
Thanks for getting this over the finish line, @gcsecsey. I pushed some final tweaks that moved selector logic into the Redux slice, but fundamentally, this was already looking good.
I think you already know this, but I'll say it anyway: long lead times between reviews and changes slow down the process as a whole, because reviewers loose context after a couple of days. RSM got in the way of this one, this is arguably a low priority fix, and I dragged my feet reviewing this on a few occasions, too. Nevertheless, something we can all keep in mind in the future 🙂
4 participants
Related issues
Fixes STU-166
How AI was used in this PR
I used AI to research the existing logout flow, then implemented and iterated on the changes using it.
Proposed Changes
When a user logged out, sync-related Redux state was not cleared, and the stale data remained in memory. If a different user logged in, they could see leftover sync UI state from the previous session.
This PR adds a userLoggedOut Redux action that is dispatched either when logging out explicitly or when an invalid token is detected. Multiple slices and a store listener react to this:
syncOperationsslice resetspullStatesandpushStatesto emptysyncslice clears the remote file tree cacheconnectedSitesslice resets modal and selection stateTesting Instructions
CleanShot.2026-03-04.at.16.41.21.mp4
Pre-merge Checklist