[codex] Track workspace transaction state#4740
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReplace sync-based "pending" logic with a per-workspace transaction store and derive create-pending state from active insert transactions; wire the store into creation hooks, optimistic actions, sidebar data, UI components, shortcuts, and the v2 workspace layout. ChangesWorkspace Transaction Tracking
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Ready to review this PR? Stage has broken it down into 5 individual chapters for you: 
Chapters generated by Stage for commit c0b29d1 on May 19, 2026 11:55pm UTC. |
Greptile SummaryThis PR fixes a bug where workspace edits (rename, task-link update) incorrectly showed the "Creating workspace" spinner by replacing the row-level
Confidence Score: 3/5The core logic is sound, but the double-write in workspaceTransactions.ts introduces a window where a stale snapshot can be observed, and the "completed"/"failed" state values exported from the store are never reliably visible to consumers. The workspaceTransactions.ts is the file that needs the most attention; layout.tsx deserves a second look for the changed guard semantics around ensureWorkspaceInSidebar and useRemoteHostStatus.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/stores/workspace-creates/workspaceTransactions.ts | New Zustand store tracking per-workspace transaction snapshots; the synchronous write + queueMicrotask double-write pattern can briefly resurrect a stale snapshot when two track() calls for the same workspaceId happen in the same synchronous run, and the "completed"/"failed" state values are immediately cleared and effectively unobservable. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsx | Replaces isSynced ($synced) guard with isCreatePending (transaction store check); ensureWorkspaceInSidebar and useRemoteHostStatus now fire sooner for cached-but-not-yet-synced pre-existing workspaces. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts | Injects pendingTransaction from the new store into sidebar workspace data and adds a useEffect to clear insert transactions on isSynced; logic is sound and dependencies look correct. |
| apps/desktop/src/renderer/routes/_authenticated/hooks/useOptimisticCollectionActions/useOptimisticCollectionActions.ts | Wires update/rename mutations through trackWorkspaceTransaction; the new fields added to PersistableTransaction are all required, and trackWorkspaceTransaction is correctly added to the useMemo dependency array. |
| apps/desktop/src/renderer/stores/workspace-creates/useWorkspaceCreates.ts | Tracks insert transactions in the new store; trackWorkspaceTransaction is added to the useCallback dependency array and called immediately after the insert transaction is created. |
Sequence Diagram
sequenceDiagram
participant UI as User Action
participant OCA as useOptimisticCollectionActions
participant WC as useWorkspaceCreates
participant Store as workspaceTransactionsStore
participant Sidebar as useDashboardSidebarData
participant Layout as V2WorkspaceLayout
participant Electric as Electric/TanStack DB
UI->>WC: create workspace
WC->>Electric: collections.v2Workspaces.insert(...)
Electric-->>WC: "transaction {id, isPersisted, mutations:[insert]}"
WC->>Store: track(workspaceId, transaction)
Store->>Store: writeSnapshot("pending") [sync]
Store->>Store: writeSnapshot("pending") [queueMicrotask]
UI->>OCA: updateWorkspace / renameWorkspace
OCA->>Electric: collections.v2Workspaces.update(...)
Electric-->>OCA: "transaction {id, isPersisted, mutations:[update]}"
OCA->>Store: track(workspaceId, transaction)
Sidebar->>Store: useWorkspaceTransactionsStore(byWorkspaceId)
Sidebar->>Sidebar: "pendingTransaction = byWorkspaceId[workspace.id]"
Sidebar->>Sidebar: "isCreatePending = type === "insert""
Electric-->>Store: isPersisted.promise resolves
Store->>Store: writeSnapshot("completed") → clear(workspaceId)
Electric-->>Layout: "workspace.$synced === true"
Layout->>Store: clearWorkspaceTransaction(workspaceId) [insert only]
Electric-->>Sidebar: "workspace.isSynced === true"
Sidebar->>Store: clearWorkspaceTransaction(workspaceId) [insert only]
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
apps/desktop/src/renderer/stores/workspace-creates/workspaceTransactions.ts:61-62
**`queueMicrotask` can resurrect a superseded transaction snapshot**
Line 61 writes the snapshot synchronously, then line 62 re-queues the same write as a microtask. If `track()` is called twice for the same `workspaceId` within a single synchronous run (e.g. `updateWorkspace` + `renameWorkspace` both invoked from the same event handler, or a rapid chain where `useOptimisticCollectionActions` triggers two mutations), the sequence becomes:
1. Sync: tx1 snapshot written for `workspaceId`
2. Sync: tx2 snapshot written for `workspaceId`
3. Microtask A (tx1): overwrites `workspaceId` with tx1 data — stale state is now live
4. Microtask B (tx2): restores correct tx2 data
Between steps 3 and 4, any Zustand subscriber that re-renders (e.g. the sidebar computing `pendingTransaction`) sees the stale tx1 snapshot. For an insert tx1 followed by an update tx2, this would incorrectly show the "Creating workspace" spinner after the user had already successfully navigated to the workspace.
The second `writeSnapshot` on line 62 appears to serve no purpose — if there is a React-batching reason for it, a comment would help; otherwise removing it eliminates the race.
### Issue 2 of 3
apps/desktop/src/renderer/stores/workspace-creates/workspaceTransactions.ts:64-77
**`"completed"` and `"failed"` states are immediately cleared and can never be observed**
`writeSnapshot("completed")` (line 67) and `get().clear(workspaceId)` (line 68) are two successive synchronous Zustand `set` calls — each one notifies subscribers before the next runs. A subscriber reading `pendingTransaction?.state` between these two calls will see `"completed"`, but any subscriber that only renders after both calls (i.e. the vast majority of real renders) will see `null` because the entry was cleared.
`WorkspaceTransactionState` exports `"completed"` and `"failed"` as first-class values, so future callers may write code that branches on them, only to find they are never reliably observable. Consider either removing these two values from the type (since clearing is the completion signal), or deferring `clear()` to the next tick so the settled state is visible for one render cycle.
### Issue 3 of 3
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsx:62-71
**`ensureWorkspaceInSidebar` now fires before Electric confirms sync for pre-existing workspaces**
Previously the guard was `!isSynced` (`workspace.$synced !== true`), which deferred the sidebar-ensure call until the server had confirmed the row. With `isCreatePending` the guard is only active while an in-flight insert transaction is tracked in the Zustand store. For workspaces that already existed before this session (loaded from cache, `$synced` still false, no transaction in the store), `isCreatePending` is immediately `false`, so `ensureWorkspaceInSidebar` fires before the Electric sync round-trip completes.
This is likely benign if `ensureWorkspaceInSidebar` is idempotent, but the same logic applies to `useRemoteHostStatus` on line 73 — it now receives a potentially stale cached workspace object and begins polling the host before the row is confirmed as current. Worth verifying the host-status hook tolerates a briefly-unsynced workspace gracefully.
Reviews (1): Last reviewed commit: "Preserve workspace update transaction st..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| writeSnapshot(transaction.state); | ||
| queueMicrotask(() => writeSnapshot(transaction.state)); |
There was a problem hiding this comment.
queueMicrotask can resurrect a superseded transaction snapshot
Line 61 writes the snapshot synchronously, then line 62 re-queues the same write as a microtask. If track() is called twice for the same workspaceId within a single synchronous run (e.g. updateWorkspace + renameWorkspace both invoked from the same event handler, or a rapid chain where useOptimisticCollectionActions triggers two mutations), the sequence becomes:
- Sync: tx1 snapshot written for
workspaceId - Sync: tx2 snapshot written for
workspaceId - Microtask A (tx1): overwrites
workspaceIdwith tx1 data — stale state is now live - Microtask B (tx2): restores correct tx2 data
Between steps 3 and 4, any Zustand subscriber that re-renders (e.g. the sidebar computing pendingTransaction) sees the stale tx1 snapshot. For an insert tx1 followed by an update tx2, this would incorrectly show the "Creating workspace" spinner after the user had already successfully navigated to the workspace.
The second writeSnapshot on line 62 appears to serve no purpose — if there is a React-batching reason for it, a comment would help; otherwise removing it eliminates the race.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/stores/workspace-creates/workspaceTransactions.ts
Line: 61-62
Comment:
**`queueMicrotask` can resurrect a superseded transaction snapshot**
Line 61 writes the snapshot synchronously, then line 62 re-queues the same write as a microtask. If `track()` is called twice for the same `workspaceId` within a single synchronous run (e.g. `updateWorkspace` + `renameWorkspace` both invoked from the same event handler, or a rapid chain where `useOptimisticCollectionActions` triggers two mutations), the sequence becomes:
1. Sync: tx1 snapshot written for `workspaceId`
2. Sync: tx2 snapshot written for `workspaceId`
3. Microtask A (tx1): overwrites `workspaceId` with tx1 data — stale state is now live
4. Microtask B (tx2): restores correct tx2 data
Between steps 3 and 4, any Zustand subscriber that re-renders (e.g. the sidebar computing `pendingTransaction`) sees the stale tx1 snapshot. For an insert tx1 followed by an update tx2, this would incorrectly show the "Creating workspace" spinner after the user had already successfully navigated to the workspace.
The second `writeSnapshot` on line 62 appears to serve no purpose — if there is a React-batching reason for it, a comment would help; otherwise removing it eliminates the race.
How can I resolve this? If you propose a fix, please make it concise.| void transaction.isPersisted.promise.then( | ||
| () => { | ||
| if (get().byWorkspaceId[workspaceId]?.id === transaction.id) { | ||
| writeSnapshot("completed"); | ||
| get().clear(workspaceId); | ||
| } | ||
| }, | ||
| () => { | ||
| if (get().byWorkspaceId[workspaceId]?.id === transaction.id) { | ||
| writeSnapshot("failed"); | ||
| get().clear(workspaceId); | ||
| } | ||
| }, | ||
| ); |
There was a problem hiding this comment.
"completed" and "failed" states are immediately cleared and can never be observed
writeSnapshot("completed") (line 67) and get().clear(workspaceId) (line 68) are two successive synchronous Zustand set calls — each one notifies subscribers before the next runs. A subscriber reading pendingTransaction?.state between these two calls will see "completed", but any subscriber that only renders after both calls (i.e. the vast majority of real renders) will see null because the entry was cleared.
WorkspaceTransactionState exports "completed" and "failed" as first-class values, so future callers may write code that branches on them, only to find they are never reliably observable. Consider either removing these two values from the type (since clearing is the completion signal), or deferring clear() to the next tick so the settled state is visible for one render cycle.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/stores/workspace-creates/workspaceTransactions.ts
Line: 64-77
Comment:
**`"completed"` and `"failed"` states are immediately cleared and can never be observed**
`writeSnapshot("completed")` (line 67) and `get().clear(workspaceId)` (line 68) are two successive synchronous Zustand `set` calls — each one notifies subscribers before the next runs. A subscriber reading `pendingTransaction?.state` between these two calls will see `"completed"`, but any subscriber that only renders after both calls (i.e. the vast majority of real renders) will see `null` because the entry was cleared.
`WorkspaceTransactionState` exports `"completed"` and `"failed"` as first-class values, so future callers may write code that branches on them, only to find they are never reliably observable. Consider either removing these two values from the type (since clearing is the completion signal), or deferring `clear()` to the next tick so the settled state is visible for one render cycle.
How can I resolve this? If you propose a fix, please make it concise.| useEffect(() => { | ||
| if ( | ||
| !workspace || | ||
| !isSynced || | ||
| isCreatePending || | ||
| lastEnsuredWorkspaceIdRef.current === workspace.id | ||
| ) | ||
| return; | ||
| lastEnsuredWorkspaceIdRef.current = workspace.id; | ||
| ensureWorkspaceInSidebar(workspace.id, workspace.projectId); | ||
| }, [ensureWorkspaceInSidebar, workspace, isSynced]); | ||
| }, [ensureWorkspaceInSidebar, workspace, isCreatePending]); |
There was a problem hiding this comment.
ensureWorkspaceInSidebar now fires before Electric confirms sync for pre-existing workspaces
Previously the guard was !isSynced (workspace.$synced !== true), which deferred the sidebar-ensure call until the server had confirmed the row. With isCreatePending the guard is only active while an in-flight insert transaction is tracked in the Zustand store. For workspaces that already existed before this session (loaded from cache, $synced still false, no transaction in the store), isCreatePending is immediately false, so ensureWorkspaceInSidebar fires before the Electric sync round-trip completes.
This is likely benign if ensureWorkspaceInSidebar is idempotent, but the same logic applies to useRemoteHostStatus on line 73 — it now receives a potentially stale cached workspace object and begins polling the host before the row is confirmed as current. Worth verifying the host-status hook tolerates a briefly-unsynced workspace gracefully.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsx
Line: 62-71
Comment:
**`ensureWorkspaceInSidebar` now fires before Electric confirms sync for pre-existing workspaces**
Previously the guard was `!isSynced` (`workspace.$synced !== true`), which deferred the sidebar-ensure call until the server had confirmed the row. With `isCreatePending` the guard is only active while an in-flight insert transaction is tracked in the Zustand store. For workspaces that already existed before this session (loaded from cache, `$synced` still false, no transaction in the store), `isCreatePending` is immediately `false`, so `ensureWorkspaceInSidebar` fires before the Electric sync round-trip completes.
This is likely benign if `ensureWorkspaceInSidebar` is idempotent, but the same logic applies to `useRemoteHostStatus` on line 73 — it now receives a potentially stale cached workspace object and begins polling the host before the row is confirmed as current. Worth verifying the host-status hook tolerates a briefly-unsynced workspace gracefully.
How can I resolve this? If you propose a fix, please make it concise.
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsx:56">
P2: The new `canUseWorkspace` condition can lock valid workspaces behind a blank state when `$synced` is false but no in-memory transaction exists (for example after transaction settle or app reload). Gate on create-pending instead of requiring an `update` transaction.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const isSynced = workspace?.$synced === true; | ||
| const canUseWorkspace = | ||
| workspace !== null && | ||
| (workspace.$synced === true || pendingTransaction?.type === "update"); |
There was a problem hiding this comment.
P2: The new canUseWorkspace condition can lock valid workspaces behind a blank state when $synced is false but no in-memory transaction exists (for example after transaction settle or app reload). Gate on create-pending instead of requiring an update transaction.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsx, line 56:
<comment>The new `canUseWorkspace` condition can lock valid workspaces behind a blank state when `$synced` is false but no in-memory transaction exists (for example after transaction settle or app reload). Gate on create-pending instead of requiring an `update` transaction.</comment>
<file context>
@@ -51,6 +51,9 @@ function V2WorkspaceLayout() {
const failedEntry = failedEntries?.[0] ?? null;
+ const canUseWorkspace =
+ workspace !== null &&
+ (workspace.$synced === true || pendingTransaction?.type === "update");
useEffect(() => {
</file context>
| (workspace.$synced === true || pendingTransaction?.type === "update"); | |
| pendingTransaction?.type !== "insert"; |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
* Track workspace transaction state * Preserve workspace update transaction state * Address workspace transaction review feedback
* Track workspace transaction state * Preserve workspace update transaction state * Address workspace transaction review feedback
1 participant
Summary
pendingTransactioninstead of deriving create loading from ``.Root Cause
`` is a row-level optimistic-state flag. It becomes false for any pending optimistic mutation, including updates, so using it as “workspace is being created” made normal workspace edits look like creates.
Validation
bun run lintbun run --cwd apps/desktop typecheckSummary by cubic
Track per-workspace transaction state to show “creating” only for inserts and keep update transactions until they finish. Insert transactions auto-clear on persistence or when
$synced === true; updates are preserved until persistence settles and no longer block workspace usage.New Features
useWorkspaceTransactionsStoreto snapshot per-workspace transactions (id, type, state, timestamps) and clear them on settle.pendingTransactionon sidebar workspace data; insert transactions clear on$synced === true, updates are not cleared by sync.Bug Fixes
!isSyncedchecks withisCreatePending(only when type isinsert), so edits no longer show the create spinner.workspaceStatus === "working".Written for commit c0b29d1. Summary will update on new commits. Review in cubic
Summary by CodeRabbit