Move pull and push states from SyncSitesProvider to a new Redux slice

[gcsecsey]

Related issues

Fixes STU-711

Proposed Changes

Sync operations are currently managed through a React Context (SyncSitesProvider) with two large custom hooks (useSyncPull and useSyncPush). This works, but it has drawbacks: polling is driven by useEffect intervals tied to the component lifecycle, state is only accessible to components wrapped in the provider, and the logic is difficult to test in isolation. The hooks mix business logic (API calls, file operations, error handling) with React state management (useState, useEffect), making them hard to reason about.

This PR replaces the Context + hooks architecture with a single Redux Toolkit slice (sync-operations-slice.ts) and listener middleware:

• New syncOperations slice (src/stores/sync/sync-operations-slice.ts): consolidates all pull/push state, async thunks for operations (pushSiteThunk, pullSiteThunk, polling thunks, cancel thunks), 22 selectors, and IPC event listeners into one file
• Listener middleware for polling (src/stores/index.ts): replaces useEffect-based polling intervals. Listeners watch for state changes and automatically dispatch polling thunks after 2s delays, creating self-sustaining loops decoupled from React component lifecycle
• Listener middleware for side effects: IPC sync with Main process (addSyncOperation/clearSyncOperation) and error modal display are now handled by listeners reacting to state changes and thunk rejections, rather than being scattered across hooks
• Deleted SyncSitesProvider and hooks (sync-sites-context.tsx, use-sync-pull.ts, use-sync-push.ts): ~1,155 lines removed
• Migrated all consuming components to use useRootSelector with selectors and dispatch with thunks instead of useSyncSites() context hook
• Extracted useLastSyncTimeText (src/hooks/sync-sites/use-last-sync-time-text.ts): small utility previously embedded in the context
• Thunk condition for dedup: polling thunks use createAsyncThunk's condition option to skip execution if the operation was cancelled, replacing manual guard checks
• Removed SyncSitesProvider from root.tsx, moved initialization to app.tsx via initializeSyncStatesThunk

Testing Instructions

This is a pure refactor, no user-facing behavior should change. The goal is to Check that everything still works exactly as before:

• Connect a local site to a WordPress.com site via the Sync tab
• Push: check that the progress UI updates through stages (creating backup, uploading with percentage, creating remote backup, applying changes, finished) and the success state appears
• Pull: Check that progress UI updates through stages (in-progress, downloading, importing, finished) and the success state appears
• Cancel: start a push or pull, then cancel it. Check that the cancelled state appears and you can clear it.
• Upload pause/resume: start a push, then disconnect the network. Check that the UI shows the paused state and resumes when the network returns.

Pre-merge Checklist

• Have you checked for TypeScript, React or other console errors?

[github-actions]

📊 Performance Test Results

Comparing 1cd2659 vs trunk

site-editor

Metric	trunk	1cd2659	Diff	Change
load	1491.00 ms	1431.00 ms	-60.00 ms	🟢 -4.0%

site-startup

Metric	trunk	1cd2659	Diff	Change
siteCreation	7127.00 ms	7079.00 ms	-48.00 ms	⚪ 0.0%
siteStartup	3947.00 ms	4931.00 ms	+984.00 ms	🔴 24.9%

---

Results are median values from multiple test runs.

Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff)

[nightnei]

Push: it seems it stucks at "Creating backup..." step. Tried a few times and also tried in trunk - worked quickly for me.

Pull: not related to the changes, since I reproduce the error in trunk too, but if you are working there - take a look please at this error when pull is finished, maybe it's a quck win:

[gcsecsey]

Push: it seems it stucks at "Creating backup..." step. Tried a few times and also tried in trunk - worked quickly for me.

Pull: not related to the changes, since I reproduce the error in trunk too, but if you are working there - take a look please at this error when pull is finished, maybe it's a quck win:

Thanks for reviewing this @nightnei! I took another look at this, and found that earlier we used a ref to keep track of the state, which let us read it back immediately after updating it. For now, I added the ref to the hooks that were previously in the usePullPushStates hook.

I'm also testing a simpler approach that updated the state objects directly where needed. If it works reliably, I'll update the PR with this approach.

[fredrikekelund]

@gcsecsey, I would challenge us to remove src/hooks/sync-sites/sync-sites-context.tsx completely as part of this refactor.

With this change, we are preserving functions like updatePushState in the React context, even though they are now essentially Redux action creators (or thunks, I guess). The logic around sync operations would be much easier to follow if we moved src/hooks/sync-sites/use-sync-push.ts and src/hooks/sync-sites/use-sync-pull.ts into src/stores/sync/sync-operations-slice.ts.

[gcsecsey]

@gcsecsey, I would challenge us to remove src/hooks/sync-sites/sync-sites-context.tsx completely as part of this refactor.

With this change, we are preserving functions like updatePushState in the React context, even though they are now essentially Redux action creators (or thunks, I guess). The logic around sync operations would be much easier to follow if we moved src/hooks/sync-sites/use-sync-push.ts and src/hooks/sync-sites/use-sync-pull.ts into src/stores/sync/sync-operations-slice.ts.

That's a good point @fredrikekelund, thanks! I'll try to move as much as possible to the Redux slice, and ping for another review.

[gcsecsey]

I carried out the changes to move most of the logic into thunks within src/stores/sync/sync-operations-slice.ts. I experimented with removing the src/hooks/sync-sites/use-sync-push.ts and src/hooks/sync-sites/use-sync-pull.ts hooks completely too, and update the components to directly use the thunks, but I found this needed quite a bit of boilerplate in each component, so for the time being, I kept the hooks as thin wrappers.

@fredrikekelund could you please take another look when you're back? Thanks!

[fredrikekelund]

Beautiful work, @gcsecsey! This has come a long way since my last review. It's looking very promising 👍

I got stuck thinking about the cancellation flow for pushSiteThunk and submitted a long suggestion there, which meant I didn't have time to complete the review. I'll pick up on Monday morning where I left off.

Here's my first batch of comments. I've also tested that pulling works as expected. I ran into some trouble with pushing, but didn't have time to debug it yet.

[fredrikekelund]

I've gone through all of the code now. Again, this is looking very promising 👍

There are a couple of themes that I've looked at, which I think should be cleaned up before landing this:

1. Some general cleanup (not exporting functions that are only used locally, removing as operators, stuff like that)
2. Error handling. There are a few opportunities to simplify this. It also ties into my next point…
3. Cancellation flows. Dealing with this is pretty gnarly. The cleaner the error handling, the easier this is to address.
4. Accepting some more repetition to gain more type safety. Listener middlewares that only run on a single action type offer us guarantees about the action payload, for example, which I think is a good tradeoff.

I've played around a lot with potential suggestion for all of these points. I've left comments and suggestions for most or all of the areas I've identified. Still, this PR will need more testing after applying those comments.

My suggestion is that I wrap up my changes locally tomorrow morning, test it, and then push those. That'll be much more efficient than us going back and forth over Github comments, @gcsecsey. Once I've pushed my changes, we'll both need to test to verify that the app works as expected before landing the PR.

Let me know what you think of that approach, @gcsecsey 🙏

[gcsecsey]

I've gone through all of the code now. Again, this is looking very promising 👍

There are a couple of themes that I've looked at, which I think should be cleaned up before landing this:

1. Some general cleanup (not exporting functions that are only used locally, removing as operators, stuff like that)
2. Error handling. There are a few opportunities to simplify this. It also ties into my next point…
3. Cancellation flows. Dealing with this is pretty gnarly. The cleaner the error handling, the easier this is to address.
4. Accepting some more repetition to gain more type safety. Listener middlewares that only run on a single action type offer us guarantees about the action payload, for example, which I think is a good tradeoff.

I've played around a lot with potential suggestion for all of these points. I've left comments and suggestions for most or all of the areas I've identified. Still, this PR will need more testing after applying those comments.

My suggestion is that I wrap up my changes locally tomorrow morning, test it, and then push those. That'll be much more efficient than us going back and forth over Github comments, @gcsecsey. Once I've pushed my changes, we'll both need to test to verify that the app works as expected before landing the PR.

Let me know what you think of that approach, @gcsecsey 🙏

Thanks @fredrikekelund for tirelessly reviewing these changes, and also for the detailed feedback for these! Yeah, that approach would be great, I'd really appreciate if you could push your changes. 🙏 We can also do a huddle to walk over everything and test it together if you want.

[fredrikekelund]

@gcsecsey, I've pushed a set of changes that revolve around what I mentioned in #2037 (review). I've tested pulling, pushing, and aborting both types of operations at different stages.

I suggest we proceed by having you test the sync features as well when you get a chance. Then, we can land the PR once we've determined there are no regressions.

[fredrikekelund]

I discovered that the ability to manually pause pushes had been lost in the diff, so I restored it. I also went ahead and added zod schemas for the API responses and restored tests that had been botched or skipped.

[fredrikekelund]

This change (and all the corresponding changes in the other files) was to appease TS. Apparently, TS interfaces can be implicitly merged, which is why the compiler was complaining in apps/studio/src/stores/index.ts when the state interfaces weren't exported. Silly stuff, but this change appeases TS.

[fredrikekelund]

I know it's not easy to see how, but e041e31 breaks sync polling, @gcsecsey. Listeners need to wait for the dispatched actions to finish, because the signal is aborted when the listener completes.

Your change might be taken from my previous suggestion. If so, know that I've already implemented everything I suggested.

[gcsecsey]

I know it's not easy to see how, but e041e31 breaks sync polling, @gcsecsey. Listeners need to wait for the dispatched actions to finish, because the signal is aborted when the listener completes.

Your change might be taken from my previous suggestion. If so, know that I've already implemented everything I suggested.

Sorry, yes I was trying to implement what you suggested earlier, I didn't realize that you also updated the listeners. I'll revert e041e31.