[codex] Reapply optimistic workspace Electric transaction work#4746
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (38)
📝 WalkthroughWalkthroughThis PR replaces in-memory workspace creation state with Electric-backed collections, substitutes ChangesWorkspace creation, UI state, and sync coordination
TRPC mutation transaction ID standardization
Electric client infrastructure and error handling
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 10 individual chapters for you: 
Chapters generated by Stage for commit 120fb8d on May 20, 2026 12:15am UTC. |
Contributor
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
Contributor
Greptile SummaryThis PR reapplies the optimistic workspace creation + Electric txid-confirmed write-sync work that was temporarily reverted for a release handoff. It replaces the previous renderer-side in-flight store with TanStack DB optimistic inserts on
Confidence Score: 3/5Safe to merge after fixing the dropped warnings regression in useWorkspaceCreates.ts. The txid plumbing, Electric error handler, and optimistic insert flow are all coherent and tests were updated accordingly. The one clear regression is that result.warnings from workspaces.create — which the old dispatch loop surfaced as toast notifications — are never accessed in the new isPersisted.promise.then handler, silently losing any warning messages the host service returns. The remaining findings (z.custom without a predicate, JWT no-op on empty token) are lower-risk quality issues. useWorkspaceCreates.ts (dropped warnings), dashboardSidebarLocal/schema.ts (unvalidated input field), collections.ts (JWT refresh silent no-op path)
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/stores/workspace-creates/useWorkspaceCreates.ts | Core refactor from async-imperative to optimistic-insert + isPersisted promise; host-service warnings are silently dropped in the new completed handler. |
| apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts | Adds onError JWT-refresh handler to all Electric shape streams and v2Workspaces onInsert that drives workspace creation; JWT refresh has a silent no-op path when the token endpoint returns an empty token. |
| apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts | Adds failedWorkspaceCreateSchema; the input field uses z.custom without a runtime predicate, so malformed localStorage data passes validation silently. |
| packages/trpc/src/router/v2-workspace/v2-workspace.ts | All mutating endpoints now wrap writes in transactions and return the Electric txid; correctly handles already-gone rows by returning txid: null. |
| packages/db/src/utils/sql.ts | getCurrentTxid now casts xid8→xid before converting to text, aligning the returned value with Electric's 32-bit xid representation. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsx | Replaces in-flight store with $synced flag and failedWorkspaceCreates collection for creating/error states; logic and state transitions are correct. |
Comments Outside Diff (3)
-
apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts, line 519 (link)z.custom<WorkspacesCreateInput>()without a predicate argument accepts any value at runtime — it only provides a TypeScript type assertion, not actual validation. If the stored localStorage entry forinputhas a stale or incompatible shape (e.g. after a schema change between app versions), the malformed data passes validation silently and will only fail later whenentry.inputis used in the retry path, with an opaque error.Prompt To Fix With AI
This is a comment left during a code review. Path: apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts Line: 519 Comment: `z.custom<WorkspacesCreateInput>()` without a predicate argument accepts any value at runtime — it only provides a TypeScript type assertion, not actual validation. If the stored localStorage entry for `input` has a stale or incompatible shape (e.g. after a schema change between app versions), the malformed data passes validation silently and will only fail later when `entry.input` is used in the retry path, with an opaque error. How can I resolve this? If you propose a fix, please make it concise.
-
apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts, line 165-179 (link)If
authClient.token()succeeds but returns a response whereresult.data?.tokenis falsy (e.g. the refresh endpoint returned an empty or null token),setJwtis not called and Electric retries with the same expired JWT, producing another 401 → anotheronErrorcall → another silent no-op. This creates an infinite retry cycle on a fully-expired session instead of surfacing the auth failure to the user.Prompt To Fix With AI
This is a comment left during a code review. Path: apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts Line: 165-179 Comment: If `authClient.token()` succeeds but returns a response where `result.data?.token` is falsy (e.g. the refresh endpoint returned an empty or null token), `setJwt` is not called and Electric retries with the same expired JWT, producing another 401 → another `onError` call → another silent no-op. This creates an infinite retry cycle on a fully-expired session instead of surfacing the auth failure to the user. How can I resolve this? If you propose a fix, please make it concise.
-
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/hooks/useDestroyDialogState/useDestroyDialogState.ts, line 140-165 (link)waitForWorkspaceDeletedtimes out,keepDeleting = trueprevents thefinallyblock from callingclearDeleting(workspaceId). There is no subsequent mechanism that callsclearDeleting— the workspace remains permanently marked as "deleting" inDeletingWorkspacesProviderfor the rest of the session. If Electric eventually delivers the deletion the sidebar item disappears and the badge becomes moot, but if sync is permanently delayed (e.g. the user stays offline), the entry leaks in the provider until restart.Prompt To Fix With AI
This is a comment left during a code review. Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/hooks/useDestroyDialogState/useDestroyDialogState.ts Line: 140-165 Comment: When `waitForWorkspaceDeleted` times out, `keepDeleting = true` prevents the `finally` block from calling `clearDeleting(workspaceId)`. There is no subsequent mechanism that calls `clearDeleting` — the workspace remains permanently marked as "deleting" in `DeletingWorkspacesProvider` for the rest of the session. If Electric eventually delivers the deletion the sidebar item disappears and the badge becomes moot, but if sync is permanently delayed (e.g. the user stays offline), the entry leaks in the provider until restart. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 4
apps/desktop/src/renderer/stores/workspace-creates/useWorkspaceCreates.ts:125-141
Warnings from the host service are silently dropped. The previous implementation iterated `result.warnings ?? []` and called `toast.warning` for each entry after a successful create. The new `isPersisted.promise.then` handler reads `metadata.result` but never touches `result.warnings`, so any host-service warnings (e.g. about agents, branch state, or quota limits) are lost without any user notification. Add `for (const w of result.warnings ?? []) toast.warning(w)` before the `return` at line 140.
### Issue 2 of 4
apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts:519
`z.custom<WorkspacesCreateInput>()` without a predicate argument accepts any value at runtime — it only provides a TypeScript type assertion, not actual validation. If the stored localStorage entry for `input` has a stale or incompatible shape (e.g. after a schema change between app versions), the malformed data passes validation silently and will only fail later when `entry.input` is used in the retry path, with an opaque error.
### Issue 3 of 4
apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts:165-179
If `authClient.token()` succeeds but returns a response where `result.data?.token` is falsy (e.g. the refresh endpoint returned an empty or null token), `setJwt` is not called and Electric retries with the same expired JWT, producing another 401 → another `onError` call → another silent no-op. This creates an infinite retry cycle on a fully-expired session instead of surfacing the auth failure to the user.
### Issue 4 of 4
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/hooks/useDestroyDialogState/useDestroyDialogState.ts:140-165
When `waitForWorkspaceDeleted` times out, `keepDeleting = true` prevents the `finally` block from calling `clearDeleting(workspaceId)`. There is no subsequent mechanism that calls `clearDeleting` — the workspace remains permanently marked as "deleting" in `DeletingWorkspacesProvider` for the rest of the session. If Electric eventually delivers the deletion the sidebar item disappears and the badge becomes moot, but if sync is permanently delayed (e.g. the user stays offline), the entry leaks in the provider until restart.
Reviews (1): Last reviewed commit: "Reapply "feat: optimistic workspace crea..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+125
to
+141
| const completed = transaction.isPersisted.promise | ||
| .then<SubmitOutcome>(() => { | ||
| const result = metadata.result; | ||
| if (!result) { | ||
| return { ok: true, workspaceId }; | ||
| } | ||
| writeWorkspacePaneLayout( | ||
| collections, | ||
| result.workspace, | ||
| result.terminals, | ||
| result.agents, | ||
| ); | ||
| if (result.workspace.id !== workspaceId) { | ||
| deleteWorkspaceLocalState(workspaceId); | ||
| } | ||
| return { ok: true, workspaceId: result.workspace.id }; | ||
| }) |
Contributor
There was a problem hiding this comment.
result.warnings ?? [] and called toast.warning for each entry after a successful create. The new isPersisted.promise.then handler reads metadata.result but never touches result.warnings, so any host-service warnings (e.g. about agents, branch state, or quota limits) are lost without any user notification. Add for (const w of result.warnings ?? []) toast.warning(w) before the return at line 140.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/stores/workspace-creates/useWorkspaceCreates.ts
Line: 125-141
Comment:
Warnings from the host service are silently dropped. The previous implementation iterated `result.warnings ?? []` and called `toast.warning` for each entry after a successful create. The new `isPersisted.promise.then` handler reads `metadata.result` but never touches `result.warnings`, so any host-service warnings (e.g. about agents, branch state, or quota limits) are lost without any user notification. Add `for (const w of result.warnings ?? []) toast.warning(w)` before the `return` at line 140.
How can I resolve this? If you propose a fix, please make it concise.
This was referenced May 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reverts #4744, which reverted #4707. This restores the TanStack DB optimistic workspace creation and Electric txid-confirmed sync work from
feat: optimistic workspace creation + Electric write-sync correctness.Why
The revert PR was needed for the release handoff. With that release path handled elsewhere, this PR reapplies the transaction work on top of current
main.Impact
v2Workspacesinserts and the failed-workspace-create local collection.Validation
bun run lint:fixbun run lintSummary by cubic
Reapplies optimistic workspace creation with Electric txid-confirmed write sync. The app now shows “Creating…” until rows are synced, and waits for confirmed deletion before clearing UI state.
New Features
v2Workspacesinserts with host create executed via mutation metadata; sync confirmation uses Electric txids.failedWorkspaceCreatescollection to surface create errors on the workspace route.$syncedasisSyncedfor “creating” state; deletion waits viawaitForWorkspaceDeleted.{ workspaceId, completed }to react to final outcome.electric-proxynow usesELECTRIC_PROTOCOL_QUERY_PARAMS.Refactors
writeWorkspacePaneLayout.creationStatusUI withisSynced; updated sidebar, title, and shortcuts logic.getCurrentTxidnow returns a numeric txid; TRPC/host-service mutations wrap writes in transactions and return txids.@electric-sql/clienttoapps/electric-proxy.Written for commit 120fb8d. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
Bug Fixes
Improvements