fix: refresh renderer stores after workspace change#491
Conversation
When the user changes workspace via SystemSection, the main process re-initializes the database but the renderer's Zustand stores still hold stale data from the old workspace. This fix emits a 'workspace:changed' IPC event from the main process after the workspace swap succeeds. The renderer subscribes to this event and clears/re-hydrates task store, message store, dashboard store, usage store, and approval store so the UI reflects the new workspace contents without requiring a manual refresh or relaunch. Fixes #404
|
Hi @HiddenPuppy, DetailsInstructions for interacting with me using comments are available here. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a state synchronization issue where the renderer's data stores retained stale information after a user switched workspaces. By introducing an IPC-based notification system, the application now correctly clears and re-initializes all relevant caches and stores, ensuring the UI reflects the data of the newly selected workspace without requiring a manual refresh. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a mechanism to notify the renderer process when a workspace change occurs, ensuring that application stores are cleared and re-hydrated with the new data. However, the current implementation only targets the first available window and ties the event listener to the lifecycle of a specific settings component. Feedback suggests broadcasting the event to all windows and moving the listener to a global location to ensure state consistency across the entire application regardless of the active view.
| useEffect(() => { | ||
| return window.clawwork.onWorkspaceChanged(async () => { | ||
| // Clear task store and re-hydrate from new DB | ||
| useTaskStore.setState({ tasks: [], activeTaskId: null, hydrated: false }); | ||
| useTaskStore.getState().hydrate(); | ||
|
|
||
| // Clear message store | ||
| useMessageStore.setState({ messagesByTask: {}, activeTurnBySession: {}, processingBySession: new Set() }); | ||
|
|
||
| // Clear dashboard, usage, and approval caches | ||
| useDashboardStore.getState().clear(); | ||
| useUsageStore.getState().clear(); | ||
| useApprovalStore.getState().clear(); | ||
|
|
||
| // Refresh settings to pick up new workspace config | ||
| await refreshSettings().catch(() => {}); | ||
| }); | ||
| }, [refreshSettings]); |
There was a problem hiding this comment.
The onWorkspaceChanged listener is currently tied to the lifecycle of the SystemSection component. This presents two architectural issues:
- Lifecycle Dependency: The stores will only refresh if the user is actively viewing the "System" settings section when the workspace change event is received. If they navigate away or close the settings before the IPC event arrives, the stores remain stale.
- Multi-window Consistency: In a multi-window setup, only the window that has the settings page open will refresh its stores. Other windows will remain out of sync.
Consider moving this subscription to a global location (e.g., a root layout component or a dedicated initialization hook) to ensure the application state is consistently updated across all windows and views. Once moved, the manual refreshSettings() call in handleChangeWorkspace (line 107) should be removed to avoid redundant network requests.
References
- Verify dependency direction and ensure architectural invariants are maintained across layers. (link)
| const win = BrowserWindow.getAllWindows()[0]; | ||
| if (win) win.webContents.send('workspace:changed', newWorkspacePath); |
There was a problem hiding this comment.
Using BrowserWindow.getAllWindows()[0] is unreliable as it only targets the first window found by the runtime. In a multi-window environment (e.g., if the user has detached chat windows or multiple instances), other windows will not receive the workspace:changed event and will continue to display stale data from the previous workspace. It is better to broadcast the event to all open windows.
| const win = BrowserWindow.getAllWindows()[0]; | |
| if (win) win.webContents.send('workspace:changed', newWorkspacePath); | |
| BrowserWindow.getAllWindows().forEach((win) => { | |
| win.webContents.send('workspace:changed', newWorkspacePath); | |
| }); |
mvanhorn
left a comment
mvanhorn
left a comment
There was a problem hiding this comment.
@HiddenPuppy I agree with gemini's two architectural concerns and think both are worth addressing before merge:
-
Lifecycle scope (SystemSection.tsx:90) - tying
onWorkspaceChangedto a component that only mounts when the user is on the System tab means navigating away during a workspace switch will silently drop the event and leave stores stale. The listener should live at a top-level provider that's always mounted (App root, an effect inside a Zustand store init, or a dedicateduseWorkspaceSynchook called from a top-level component). -
Multi-window broadcast (workspace-handlers.ts:60) -
getAllWindows()[0]is fine for the single-window case but breaks for detached/secondary windows.BrowserWindow.getAllWindows().forEach(w => w.webContents.send(...))covers it.
The quality CI failure looks unrelated - 6 no-explicit-any errors in packages/desktop/test/ssrf-guard.test.ts, which this PR doesn't touch. Worth checking whether main is also red on that file.
Overall direction is right - clearing the renderer caches after a workspace switch is the correct fix for #404. Just want the listener and broadcast wired more robustly.
2 participants
Summary
When the user changes workspace via SystemSection, the main process re-initializes the database for the new path, but the renderer's Zustand stores (task, message, dashboard, usage, approval) still hold stale data from the old workspace. The user would see a blank or incorrect task list and would need to manually refresh or relaunch.
Changes
workspace-handlers.ts): After reinitDatabase + updateConfig succeed, emit aworkspace:changedIPC event to the renderer window with the new workspace pathindex.ts+clawwork.d.ts): ExposeonWorkspaceChanged(callback)that subscribes to the IPC event and returns an unsubscribe functionSystemSection.tsx): A useEffect subscribes to onWorkspaceChanged. On receipt:Testing
pnpm typecheck— passespnpm test— all 311 tests pass (49 test files)Fixes #404