feat: optimistic workspace creation + Electric write-sync correctness#4707
Merged
Conversation
Electric collections had no shapeOptions.onError, so a non-retryable
error (expired-JWT 401, 409 shape-expired, 4xx) permanently stopped the
stream — that collection silently stopped syncing until app restart.
Add a shared handleElectricSyncError wired to every collection: returns
{} to retry (ShapeStream backoff paces it), and refreshes the JWT on a
401 so the retry uses a fresh token.
The delete dialog now awaits waitForWorkspaceDeleted — the workspace row leaving the local Electric collection — before clearing the deleting state. If sync is slow it keeps the row hidden and warns, rather than re-enabling stale actions on a half-deleted workspace.
|
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. |
|
Ready to review this PR? Stage has broken it down into 12 individual chapters for you: 
Chapters generated by Stage for commit 3d00569 on May 19, 2026 8:32pm UTC. |
Contributor
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR replaces the renderer-side in-flight workspace-creation Zustand store with Electric-backed persistence: workspaces now track sync status via an ChangesWorkspace creation flow migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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)
|
Contributor
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
Contributor
Greptile SummaryThis PR rewrites workspace creation onto TanStack DB optimistic inserts, fixes Electric write-path correctness (xid8→xid cast, no-op mutation txid suppression, protocol-param constant), adds
Confidence Score: 3/5The broad rewrite is type-checked and lint-clean, but has not had manual E2E validation and contains a stuck-deleting-state defect in the timeout path. The workspace deletion flow has a real behavioral gap: when useDestroyDialogState.ts (keepDeleting never clears), host-service workspaces.ts (unsafe CloudWorkspace cast for txid), v2-project.ts (undefined-spread dead-code path)
|
| Filename | Overview |
|---|---|
| packages/db/src/utils/sql.ts | Fixes getCurrentTxid to cast xid8 to xid (32-bit) before parsing, and validates with Number.isSafeInteger; corrects the root cause of Electric txid-timeout divergence |
| apps/electric-proxy/src/electric.ts | Replaces hand-maintained PROTOCOL_PARAMS set with ELECTRIC_PROTOCOL_QUERY_PARAMS from @electric-sql/client; clean improvement with no functional risk |
| apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts | Adds onInsert for v2Workspaces (calls host-service create), handleElectricSyncError wired to all 24 collections, failedWorkspaceCreates local-storage collection, and electricTxidMatch guard for no-op mutations; broad surface area but mechanically sound |
| apps/desktop/src/renderer/stores/workspace-creates/useWorkspaceCreates.ts | Rewrites submit() as synchronous fire-and-forget returning { workspaceId, completed }; inserts an optimistic row and stores failures in failedWorkspaceCreates; replaces heavyweight Zustand sidecar cleanly |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/hooks/useDestroyDialogState/useDestroyDialogState.ts | Adds waitForWorkspaceDeleted after destroy to confirm Electric sync; clearDeleting is permanently skipped when the 30s timeout fires, leaving the workspace locked in deleting state for the session lifetime |
| packages/trpc/src/router/v2-workspace/v2-workspace.ts | Wraps all mutating endpoints in transactions, adds .returning() guards to skip getCurrentTxid on no-ops, and surfaces txid on create/update/delete responses; logic is correct, posthog skipped for already-gone deletes as expected |
| packages/host-service/src/trpc/router/workspaces/workspaces.ts | Adds extractCreateTxid to surface the cloud txid for Electric matching, but uses an unsafe cast because CloudWorkspace lacks txid in its TypeScript type despite the router now returning it |
| apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/workspaceSyncWaits.ts | New waitForWorkspaceDeleted utility; subscription + synchronous final check with settled guard correctly handles races; timeout and cleanup logic look solid |
| packages/trpc/src/router/v2-project/v2-project.ts | Wraps project create in a transaction to capture txid; the !inserted dead-code path spreads undefined producing a typeless return value without explicit error |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsx | Simplified to a clean 4-way tree: synced to real UI, unsynced to creating, failed entry to error state, missing to not found; ensureWorkspaceInSidebar now guarded on isSynced to avoid acting on optimistic rows |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["submit(args)"] --> B["Clear failedWorkspaceCreates entry"]
B --> C["v2Workspaces.insert(optimisticRow, metadata)"]
C --> D["writeWorkspacePaneLayout no launches"]
D --> E["Return workspaceId and completed Promise"]
E --> F["User navigates to /v2-workspace/:id"]
C --> G["onInsert fires async"]
G --> H["host-service workspaces.create"]
H -->|success| I["metadata.result = result, return electricTxidMatch"]
H -->|failure| J["throw - TanStack DB rolls back optimistic row"]
I --> K["Electric stream delivers row, isPersisted resolves"]
K --> L["writeWorkspacePaneLayout with terminals and agents"]
L --> M["completed ok: true"]
J --> N["completed .catch fires"]
N --> O["recordFailure - failedWorkspaceCreates.insert"]
O --> P["Layout: WorkspaceCreateErrorState"]
F --> Q{workspace state}
Q -->|synced true| R["Real workspace UI"]
Q -->|synced false| S["WorkspaceCreatingState"]
Q -->|no row with failed entry| P
Q -->|no row no failure| T["WorkspaceNotFoundState"]
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/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/hooks/useDestroyDialogState/useDestroyDialogState.ts:143-176
**Deleting state permanently leaked on slow-sync timeout**
When `waitForWorkspaceDeleted` times out, `keepDeleting` is set to `true`, `onDeleted()` navigates the user away, and the function returns early. The `finally` block then skips `clearDeleting(workspaceId)`. For the session lifetime, `DeletingWorkspacesProvider` considers this workspace as still-deleting.
In the happy path (Electric eventually delivers the row removal) this is harmless. But if the row persists — e.g. Electric is down and the delete is not durably confirmed — the workspace reappears in the sidebar permanently locked in the deleting state, with no in-app way to recover short of restarting. The user also can't retry the delete because `isDeleting` guards the destroy action.
### Issue 2 of 3
packages/host-service/src/trpc/router/workspaces/workspaces.ts:117-122
**Unsafe cast bypasses TypeScript type-checking for `txid`**
`(row as { txid?: unknown })` is needed only because `CloudWorkspace` — inferred from the tRPC output type of `v2WorkspaceRouter.create` — does not include `txid` in its TypeScript type, even though the field is present at runtime. This means the compiler will not catch future type mismatches between the router's return shape and this access site. Since `v2-workspace.ts` was updated to return `{ ...result.workspace, txid: result.txid }`, the inferred type should include `txid: number | null`; if it doesn't, the tRPC type for the `create` endpoint may need regenerating or the `CloudWorkspace` alias should be tightened to use the actual router output type directly.
### Issue 3 of 3
packages/trpc/src/router/v2-project/v2-project.ts:220-315
**`{ ...undefined, txid }` returns an incomplete object when `inserted` is falsy**
When `!inserted`, `result.project` is `undefined`, and the outer `return { ...project, txid }` spreads `undefined` — producing `{ txid: null }` without any project fields. TypeScript allows this because the declared type of `project` is `typeof v2Projects.$inferSelect | undefined` and spreading `undefined` is a no-op. Any caller that destructures project properties from the return value would silently receive `undefined`. In practice `INSERT ... RETURNING` always either returns the row or throws, so the branch is dead code, but making the dead-code path explicit (e.g. `throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" })`) would prevent this from becoming a silent data hole if the assumption ever breaks.
Reviews (1): Last reviewed commit: "feat(desktop): confirm workspace deletio..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
143
to
176
| @@ -151,7 +169,9 @@ export function useDestroyDialogState({ | |||
| ); | |||
| } | |||
| } finally { | |||
| clearDeleting(workspaceId); | |||
| if (!keepDeleting) { | |||
| clearDeleting(workspaceId); | |||
| } | |||
| inFlight.current = false; | |||
| } | |||
Contributor
There was a problem hiding this comment.
When waitForWorkspaceDeleted times out, keepDeleting is set to true, onDeleted() navigates the user away, and the function returns early. The finally block then skips clearDeleting(workspaceId). For the session lifetime, DeletingWorkspacesProvider considers this workspace as still-deleting.
In the happy path (Electric eventually delivers the row removal) this is harmless. But if the row persists — e.g. Electric is down and the delete is not durably confirmed — the workspace reappears in the sidebar permanently locked in the deleting state, with no in-app way to recover short of restarting. The user also can't retry the delete because isDeleting guards the destroy action.
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: 143-176
Comment:
**Deleting state permanently leaked on slow-sync timeout**
When `waitForWorkspaceDeleted` times out, `keepDeleting` is set to `true`, `onDeleted()` navigates the user away, and the function returns early. The `finally` block then skips `clearDeleting(workspaceId)`. For the session lifetime, `DeletingWorkspacesProvider` considers this workspace as still-deleting.
In the happy path (Electric eventually delivers the row removal) this is harmless. But if the row persists — e.g. Electric is down and the delete is not durably confirmed — the workspace reappears in the sidebar permanently locked in the deleting state, with no in-app way to recover short of restarting. The user also can't retry the delete because `isDeleting` guards the destroy action.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
117
to
+122
| > | ||
| >; | ||
|
|
||
| function extractCreateTxid(row: CloudWorkspace): number | null { | ||
| const txid = (row as { txid?: unknown }).txid; | ||
| return typeof txid === "number" ? txid : null; |
Contributor
There was a problem hiding this comment.
txid
(row as { txid?: unknown }) is needed only because CloudWorkspace — inferred from the tRPC output type of v2WorkspaceRouter.create — does not include txid in its TypeScript type, even though the field is present at runtime. This means the compiler will not catch future type mismatches between the router's return shape and this access site. Since v2-workspace.ts was updated to return { ...result.workspace, txid: result.txid }, the inferred type should include txid: number | null; if it doesn't, the tRPC type for the create endpoint may need regenerating or the CloudWorkspace alias should be tightened to use the actual router output type directly.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/trpc/router/workspaces/workspaces.ts
Line: 117-122
Comment:
**Unsafe cast bypasses TypeScript type-checking for `txid`**
`(row as { txid?: unknown })` is needed only because `CloudWorkspace` — inferred from the tRPC output type of `v2WorkspaceRouter.create` — does not include `txid` in its TypeScript type, even though the field is present at runtime. This means the compiler will not catch future type mismatches between the router's return shape and this access site. Since `v2-workspace.ts` was updated to return `{ ...result.workspace, txid: result.txid }`, the inferred type should include `txid: number | null`; if it doesn't, the tRPC type for the `create` endpoint may need regenerating or the `CloudWorkspace` alias should be tightened to use the actual router output type directly.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
220
to
+315
| @@ -300,7 +312,7 @@ export const v2ProjectRouter = { | |||
| })(); | |||
| } | |||
|
|
|||
| return project; | |||
| return { ...project, txid }; | |||
Contributor
There was a problem hiding this comment.
{ ...undefined, txid } returns an incomplete object when inserted is falsy
When !inserted, result.project is undefined, and the outer return { ...project, txid } spreads undefined — producing { txid: null } without any project fields. TypeScript allows this because the declared type of project is typeof v2Projects.$inferSelect | undefined and spreading undefined is a no-op. Any caller that destructures project properties from the return value would silently receive undefined. In practice INSERT ... RETURNING always either returns the row or throws, so the branch is dead code, but making the dead-code path explicit (e.g. throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" })) would prevent this from becoming a silent data hole if the assumption ever breaks.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/trpc/src/router/v2-project/v2-project.ts
Line: 220-315
Comment:
**`{ ...undefined, txid }` returns an incomplete object when `inserted` is falsy**
When `!inserted`, `result.project` is `undefined`, and the outer `return { ...project, txid }` spreads `undefined` — producing `{ txid: null }` without any project fields. TypeScript allows this because the declared type of `project` is `typeof v2Projects.$inferSelect | undefined` and spreading `undefined` is a no-op. Any caller that destructures project properties from the return value would silently receive `undefined`. In practice `INSERT ... RETURNING` always either returns the row or throws, so the branch is dead code, but making the dead-code path explicit (e.g. `throw new TRPCError({ code: "INTERNAL_SERVER_ERROR" })`) would prevent this from becoming a silent data hole if the assumption ever breaks.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/trpc/src/router/v2-workspace/v2-workspace.ts (1)
576-579:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNormalize idempotent delete responses to always include
txid.Both early
alreadyGonebranches omittxid, while other delete branches return it. Keep the response shape stable by returningtxid: nullthere too.Suggested patch
- if (!workspace) { - // Already gone in the cloud; idempotent success. - return { success: true, alreadyGone: true as const }; - } + if (!workspace) { + // Already gone in the cloud; idempotent success. + return { success: true, alreadyGone: true as const, txid: null }; + } ... - if (!workspace) { - return { success: true, alreadyGone: true as const }; - } + if (!workspace) { + return { success: true, alreadyGone: true as const, txid: null }; + }Also applies to: 635-637
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/trpc/src/router/v2-workspace/v2-workspace.ts` around lines 576 - 579, Early idempotent delete branches that return { success: true, alreadyGone: true as const } omit txid, causing inconsistent response shape; update those early return sites (the checks on the workspace variable inside the delete handler in v2-workspace.ts) to return { success: true, alreadyGone: true as const, txid: null } so they match the other delete branches that include txid; apply the same change to both occurrences around the existing workspace checks (the two alreadyGone return points referenced in the review).
🧹 Nitpick comments (2)
apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts (1)
242-256: 💤 Low valueConsider preventing retry on persistent auth failure.
When JWT refresh fails (line 249-251), the function still returns
{}which signals Electric to retry the stream. If the auth issue persists, this could cause continuous retry attempts with an invalid token.Consider returning
undefinedor a signal to stop retrying when refresh definitively fails, or implementing exponential backoff/max retry limits at a higher level.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts` around lines 242 - 256, The handler handleElectricSyncError currently returns {} after a failed JWT refresh which signals Electric to retry; change it so that when authClient.token() fails (inside the catch in handleElectricSyncError) you return undefined (or another stop-retry sentinel) instead of {} to prevent immediate retries with an invalid token; keep the existing setJwt call when refresh succeeds, and only return {} for non-auth errors as before, ensuring handleElectricSyncError short-circuits retry on definitive auth refresh failure.apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/workspaceSyncWaits.ts (1)
14-38: ⚡ Quick winConsider defensive refactoring for subscription cleanup timing.
The code at lines 35-38 relies on explicit callback guarding via the
settledflag. While this works correctly, the pattern of accessingsubscriptionin callbacks defined before assignment could be hardened. Adding a nullable subscription wrapper and shared cleanup function (as shown in the diff) eliminates theoretical callback-timing brittleness and makes cleanup logic DRY.Proposed defensive refactor
export function waitForWorkspaceDeleted( collection: AppCollections["v2Workspaces"], workspaceId: string, ): Promise<void> { if (!collection.get(workspaceId)) { return Promise.resolve(); } return new Promise((resolve, reject) => { let settled = false; + let subscription: { unsubscribe: () => void } | null = null; + const cleanup = () => { + clearTimeout(timeoutId); + subscription?.unsubscribe(); + }; + const timeoutId = setTimeout(() => { if (settled) return; settled = true; - subscription.unsubscribe(); + cleanup(); reject( new Error( `Workspace ${workspaceId} deletion did not sync to the local collection`, ), ); }, ELECTRIC_WRITE_SYNC_TIMEOUT_MS); const finish = () => { if (settled || collection.get(workspaceId)) return; settled = true; - clearTimeout(timeoutId); - subscription.unsubscribe(); + cleanup(); resolve(); }; - const subscription = collection.subscribeChanges(finish, { - includeInitialState: false, - }); - finish(); + try { + subscription = collection.subscribeChanges(finish, { + includeInitialState: false, + }); + finish(); + } catch (err) { + if (settled) return; + settled = true; + cleanup(); + reject(err instanceof Error ? err : new Error(String(err))); + } }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/workspaceSyncWaits.ts` around lines 14 - 38, The current promise uses a `settled` flag and callbacks (`finish` and the timeout) that reference `subscription` before it's assigned, which is brittle; refactor by introducing a nullable `let activeSubscription: Unsubscribe | null = null` (or similar) and a single shared `cleanup` function that checks `settled`, clears the timeout, unsubscribes `activeSubscription` if non-null, and resolves/rejects as needed; assign `activeSubscription = collection.subscribeChanges(finish, { includeInitialState: false })` and have both the timeout handler and `finish` call the shared `cleanup` to remove duplication and eliminate timing race conditions (reference symbols: `subscription`, `finish`, `settled`, `collection.subscribeChanges`, `ELECTRIC_WRITE_SYNC_TIMEOUT_MS`, `workspaceId`).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/components/WorkspaceCreateErrorState/WorkspaceCreateErrorState.tsx`:
- Around line 22-24: The retry handler currently calls submit(...) but never
removes the failed entry from failedWorkspaceCreates, leaving orphaned state and
stale UI; update handleRetry to first remove the current entry from
failedWorkspaceCreates (use the same identifier used in storage, e.g., entry.id
or entry.hostId via the existing removal helper or a new
removeFailedWorkspaceCreate function), then call submit({ hostId: entry.hostId,
snapshot: entry.input }) and proceed with the existing navigation flow so the UI
no longer shows the failed entry after a successful retry.
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx`:
- Line 238: The toast.error call currently passes a plain string
(`toast.error(\`Failed to import ${wt.branch}: ${outcome.error}\`)`) which is
not selectable due to renderer global styles; change it to pass a React element
with selectable classes so users can copy the error text. Replace the string
argument to toast.error with a JSX element (e.g., a <span> or <div>) that
contains the same interpolated text and has className="select-text cursor-text",
keeping the same variables (wt.branch and outcome.error) and preserving existing
behavior of the toast invocation.
---
Outside diff comments:
In `@packages/trpc/src/router/v2-workspace/v2-workspace.ts`:
- Around line 576-579: Early idempotent delete branches that return { success:
true, alreadyGone: true as const } omit txid, causing inconsistent response
shape; update those early return sites (the checks on the workspace variable
inside the delete handler in v2-workspace.ts) to return { success: true,
alreadyGone: true as const, txid: null } so they match the other delete branches
that include txid; apply the same change to both occurrences around the existing
workspace checks (the two alreadyGone return points referenced in the review).
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts`:
- Around line 242-256: The handler handleElectricSyncError currently returns {}
after a failed JWT refresh which signals Electric to retry; change it so that
when authClient.token() fails (inside the catch in handleElectricSyncError) you
return undefined (or another stop-retry sentinel) instead of {} to prevent
immediate retries with an invalid token; keep the existing setJwt call when
refresh succeeds, and only return {} for non-auth errors as before, ensuring
handleElectricSyncError short-circuits retry on definitive auth refresh failure.
In
`@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/workspaceSyncWaits.ts`:
- Around line 14-38: The current promise uses a `settled` flag and callbacks
(`finish` and the timeout) that reference `subscription` before it's assigned,
which is brittle; refactor by introducing a nullable `let activeSubscription:
Unsubscribe | null = null` (or similar) and a single shared `cleanup` function
that checks `settled`, clears the timeout, unsubscribes `activeSubscription` if
non-null, and resolves/rejects as needed; assign `activeSubscription =
collection.subscribeChanges(finish, { includeInitialState: false })` and have
both the timeout handler and `finish` call the shared `cleanup` to remove
duplication and eliminate timing race conditions (reference symbols:
`subscription`, `finish`, `settled`, `collection.subscribeChanges`,
`ELECTRIC_WRITE_SYNC_TIMEOUT_MS`, `workspaceId`).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 68f3c76f-8a74-48a8-a401-0565ba71b463
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (37)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/hooks/useDestroyDialogState/useDestroyDialogState.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/DashboardSidebarWorkspaceItem.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarCollapsedWorkspaceButton/DashboardSidebarCollapsedWorkspaceButton.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarExpandedWorkspaceRow/DashboardSidebarExpandedWorkspaceRow.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceIcon/DashboardSidebarWorkspaceIcon.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/utils/getCreationStatusText.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/utils/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarShortcuts/useDashboardSidebarShortcuts.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/types.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/V2WorkspaceTitle/V2WorkspaceTitle.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/PropertiesSidebar/components/OpenInWorkspaceV2/OpenInWorkspaceV2.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopoverV2/RunInWorkspacePopoverV2.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunIssuesInWorkspacePopover/RunIssuesInWorkspacePopover.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/components/WorkspaceCreateErrorState/WorkspaceCreateErrorState.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsxapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/hooks/useBranchPickerController/useBranchPickerController.tsapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/hooks/useSubmitWorkspace/useSubmitWorkspace.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/workspaceSyncWaits.tsapps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsxapps/desktop/src/renderer/stores/workspace-creates/Manager.tsxapps/desktop/src/renderer/stores/workspace-creates/index.tsapps/desktop/src/renderer/stores/workspace-creates/store.tsapps/desktop/src/renderer/stores/workspace-creates/useWorkspaceCreates.tsapps/desktop/src/renderer/stores/workspace-creates/writeWorkspacePaneLayout.tsapps/electric-proxy/package.jsonapps/electric-proxy/src/electric.tspackages/db/src/utils/sql.tspackages/host-service/src/trpc/router/workspaces/workspaces.tspackages/trpc/src/router/chat/chat.tspackages/trpc/src/router/v2-host/v2-host.tspackages/trpc/src/router/v2-project/v2-project.test.tspackages/trpc/src/router/v2-project/v2-project.tspackages/trpc/src/router/v2-workspace/v2-workspace.ts
💤 Files with no reviewable changes (5)
- apps/desktop/src/renderer/stores/workspace-creates/store.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/utils/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/utils/getCreationStatusText.ts
- apps/desktop/src/renderer/stores/workspace-creates/Manager.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx
Comment on lines
+22
to
+24
| const handleRetry = () => { | ||
| submit({ hostId: entry.hostId, snapshot: entry.input }); | ||
| }; |
Contributor
There was a problem hiding this comment.
Retry does not remove the failed entry from failedWorkspaceCreates.
When the user clicks "Try again", handleRetry calls submit() but doesn't remove the current entry from failedWorkspaceCreates. This means:
- If retry succeeds, the failed entry remains in storage (orphaned data)
- The user stays on the error page viewing stale state
Consider removing the failed entry before submitting the retry and navigating to the new workspace.
🐛 Proposed fix
const handleRetry = () => {
+ collections.failedWorkspaceCreates.delete(entry.id);
- submit({ hostId: entry.hostId, snapshot: entry.input });
+ const { workspaceId } = submit({ hostId: entry.hostId, snapshot: entry.input });
+ void navigate({ to: "/v2-workspace/$workspaceId", params: { workspaceId } });
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/components/WorkspaceCreateErrorState/WorkspaceCreateErrorState.tsx`
around lines 22 - 24, The retry handler currently calls submit(...) but never
removes the failed entry from failedWorkspaceCreates, leaving orphaned state and
stale UI; update handleRetry to first remove the current entry from
failedWorkspaceCreates (use the same identifier used in storage, e.g., entry.id
or entry.hostId via the existing removal helper or a new
removeFailedWorkspaceCreate function), then call submit({ hostId: entry.hostId,
snapshot: entry.input }) and proceed with the existing navigation flow so the UI
no longer shows the failed entry after a successful retry.
| ensureWorkspaceInSidebar(outcome.workspaceId, v2ProjectId); | ||
| } else { | ||
| toast.error(`Failed to import ${wt.branch}: ${result.error}`); | ||
| toast.error(`Failed to import ${wt.branch}: ${outcome.error}`); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx | sed -n '230,250p'Repository: superset-sh/superset
Length of output: 844
🏁 Script executed:
rg "toast\." apps/desktop/src/renderer --type tsx --type ts -A 2 -B 2 | head -100Repository: superset-sh/superset
Length of output: 92
🏁 Script executed:
rg "toast\.error" apps/desktop/src/renderer -A 1 -B 1 | head -80Repository: superset-sh/superset
Length of output: 8355
🏁 Script executed:
grep -n "toast" apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx | head -5Repository: superset-sh/superset
Length of output: 322
🏁 Script executed:
cat -n apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx | sed -n '208,220p'Repository: superset-sh/superset
Length of output: 500
🏁 Script executed:
rg "toast\.(error|success|warning|info)\(" apps/desktop/src/renderer --multiline -A 3 | rg "<" | head -20Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
rg "from.*sonner" apps/desktop/src/rendererRepository: superset-sh/superset
Length of output: 23263
🏁 Script executed:
find . -path "*/node_modules" -prune -o -name "sonner*" -type f -print 2>/dev/null | head -20Repository: superset-sh/superset
Length of output: 107
🏁 Script executed:
cat packages/ui/src/components/ui/sonner.tsxRepository: superset-sh/superset
Length of output: 1252
🏁 Script executed:
rg "user-select.*none" apps/desktop/src/renderer --type css --type tsx --type ts -iRepository: superset-sh/superset
Length of output: 92
🏁 Script executed:
rg "user-select.*none" apps/desktop/src/renderer -iRepository: superset-sh/superset
Length of output: 817
🏁 Script executed:
cat apps/desktop/src/renderer/globals.css | grep -A 5 -B 5 "user-select"Repository: superset-sh/superset
Length of output: 334
🏁 Script executed:
rg "select-text cursor-text" apps/desktop/src/rendererRepository: superset-sh/superset
Length of output: 8098
Make the new toast error message selectable.
Line 238 adds user-facing error text without explicit selectable-text classes. Please render it with select-text cursor-text so users can copy details.
Suggested change
- toast.error(`Failed to import ${wt.branch}: ${outcome.error}`);
+ toast.error(
+ <span className="select-text cursor-text">
+ {`Failed to import ${wt.branch}: ${outcome.error}`}
+ </span>,
+ );As per coding guidelines, apps/desktop/**/*.{tsx,jsx}: Error text must be selectable by users with explicit select-text cursor-text classes; renderer sets user-select: none on body.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| toast.error(`Failed to import ${wt.branch}: ${outcome.error}`); | |
| toast.error( | |
| <span className="select-text cursor-text"> | |
| {`Failed to import ${wt.branch}: ${outcome.error}`} | |
| </span>, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx`
at line 238, The toast.error call currently passes a plain string
(`toast.error(\`Failed to import ${wt.branch}: ${outcome.error}\`)`) which is
not selectable due to renderer global styles; change it to pass a React element
with selectable classes so users can copy the error text. Replace the string
argument to toast.error with a JSX element (e.g., a <span> or <div>) that
contains the same interpolated text and has className="select-text cursor-text",
keeping the same variables (wt.branch and outcome.error) and preserving existing
behavior of the toast invocation.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 38 files
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/stores/workspace-creates/writeWorkspacePaneLayout.ts">
<violation number="1" location="apps/desktop/src/renderer/stores/workspace-creates/writeWorkspacePaneLayout.ts:59">
P2: Ensure the sidebar project record exists before inserting workspace local state; otherwise newly created workspaces can be filtered out from sidebar rendering.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| .filter((item) => item.projectId === projectId) | ||
| .map((item) => ({ tabOrder: item.tabOrder })), | ||
| ]; | ||
| collections.v2WorkspaceLocalState.insert({ |
Contributor
There was a problem hiding this comment.
P2: Ensure the sidebar project record exists before inserting workspace local state; otherwise newly created workspaces can be filtered out from sidebar rendering.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/stores/workspace-creates/writeWorkspacePaneLayout.ts, line 59:
<comment>Ensure the sidebar project record exists before inserting workspace local state; otherwise newly created workspaces can be filtered out from sidebar rendering.</comment>
<file context>
@@ -0,0 +1,74 @@
+ .filter((item) => item.projectId === projectId)
+ .map((item) => ({ tabOrder: item.tabOrder })),
+ ];
+ collections.v2WorkspaceLocalState.insert({
+ workspaceId: workspace.id,
+ createdAt: new Date(),
</file context>
useWorkspaceHostTarget resolved a host URL for any row present in the v2Workspaces collection, including optimistic create rows. That made useDiffStats fire git.getStatus against a host that has not registered the workspace yet, returning NOT_FOUND on every create. Gate host resolution on $synced so host-backed hooks stay disabled until sync. Also exclude in-flight deletes from sidebar jump shortcuts: a deleting row stays $synced until Electric removes it, so hotkeys could navigate into a workspace whose host record was already destroyed.
When a create retry resolves to a different (canonical) workspace id, the optimistic row for the original id rolls back and the route is left on a workspace that no longer exists — WorkspaceNotFoundState. Await the submit outcome and navigate to the canonical id when it differs.
v2Project.create inserts via tx.insert() inside a transaction, so expect(dbInsert).not.toHaveBeenCalled() was vacuously true. Assert on txInsert so the "rejected before insert" tests actually verify it.
saddlepaddle
added a commit
that referenced
this pull request
May 20, 2026
sazabi Bot
pushed a commit
that referenced
this pull request
May 20, 2026
…#4707) * WIP * WIP * fix(desktop): retry Electric shape streams on sync errors Electric collections had no shapeOptions.onError, so a non-retryable error (expired-JWT 401, 409 shape-expired, 4xx) permanently stopped the stream — that collection silently stopped syncing until app restart. Add a shared handleElectricSyncError wired to every collection: returns {} to retry (ShapeStream backoff paces it), and refreshes the JWT on a 401 so the retry uses a fresh token. * feat(desktop): confirm workspace deletion synced before clearing state The delete dialog now awaits waitForWorkspaceDeleted — the workspace row leaving the local Electric collection — before clearing the deleting state. If sync is slow it keeps the row hidden and warns, rather than re-enabling stale actions on a half-deleted workspace. * fix(desktop): treat unsynced workspaces as non-actionable useWorkspaceHostTarget resolved a host URL for any row present in the v2Workspaces collection, including optimistic create rows. That made useDiffStats fire git.getStatus against a host that has not registered the workspace yet, returning NOT_FOUND on every create. Gate host resolution on $synced so host-backed hooks stay disabled until sync. Also exclude in-flight deletes from sidebar jump shortcuts: a deleting row stays $synced until Electric removes it, so hotkeys could navigate into a workspace whose host record was already destroyed. * fix(desktop): redirect to canonical workspace after create retry When a create retry resolves to a different (canonical) workspace id, the optimistic row for the original id rolls back and the route is left on a workspace that no longer exists — WorkspaceNotFoundState. Await the submit outcome and navigate to the canonical id when it differs. * test(trpc): assert tx.insert in v2-project create reject tests v2Project.create inserts via tx.insert() inside a transaction, so expect(dbInsert).not.toHaveBeenCalled() was vacuously true. Assert on txInsert so the "rejected before insert" tests actually verify it.
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
Reworks workspace creation onto TanStack DB optimistic inserts with Electric txid-confirmed sync, and fixes the write-path correctness gaps underneath it. Four parts:
getCurrentTxid()xid cast, electric-proxy protocol-param constant, and no-op mutations no longer fabricate txids.v2Workspacesinsert + a synchronous fire-and-forgetsubmit(); the in-flight Zustand sidecar (workspace-createsstore +Manager) is gone, replaced by a minimalfailedWorkspaceCreatescollection that only holds failures.onError— Electric streams now retry (and refresh the JWT on 401) instead of silently dying on the first non-retryable error.Relationship to #4632
This is an alternative to the open Electric-team PR #4632. It keeps that PR's write-path correctness fixes and the optimistic-insert direction, but preserves the in-product retry UI — #4632 removes
WorkspaceCreateErrorState/WorkspaceCreatingState; here they're kept and wired to a real retry mechanism — and keeps in-flight creates visible in the sidebar as loading rows. It also adds shape-streamonError(Part 3), a read-path gap #4632 doesn't touch.Part 1: Electric write-path correctness
Why
getCurrentTxid()returnedpg_current_xact_id()::text— a 64-bitxid8. Electric receives the 32-bitxidfrom logical replication. The two agree on a fresh database but diverge once the xid epoch advances, after whichawaitTxIdcan never match and every optimistic write times out. Separately, no-op writes (raced deletes, no-change updates) returned a freshly-minted txid that Electric will never emit downstream, so the renderer waited the full 30s sync timeout for nothing.What
getCurrentTxid()→pg_current_xact_id()::xid::text, parsed andNumber.isSafeInteger-validated.ELECTRIC_PROTOCOL_QUERY_PARAMSconstant from@electric-sql/clientinstead of a hand-maintainedPROTOCOL_PARAMSset (so new protocol params don't silently get dropped).v2-workspace/v2-host/v2-project/chatrouters gategetCurrentTxid()behind.returning()/ affected-row checks — no-ops return no txid (ornull), missing rows throwNOT_FOUND.workspaces.createsurfaces the cloudtxid(extractCreateTxid) so the renderer can hand it back to Electric.Part 2: Workspace create-flow rewrite
Why
Workspace creation ran through a heavyweight renderer-memory sidecar — the
workspace-createsZustand store plus aManagercomponent reconciling it against Electric. It modeled in-flight rows outside TanStack DB, duplicating state the collection already tracks, and was hard to reason about.What / How
submit()is now synchronous fire-and-forget — it returns{ workspaceId, completed: Promise<SubmitOutcome> }. It does an optimisticcollections.v2Workspaces.insert()(carrying a create-mutation metadata payload) and writes the pane layout up-front;v2Workspaces.onInsertcalls host-serviceworkspaces.createand returns the resulting txid for Electric to match.$synced: false) and succeeded ($synced: true). The one state it can't model is failed — the optimistic row rolls back on error — so a small localStorage-backedfailedWorkspaceCreatescollection holds failures, and the route'sWorkspaceCreateErrorStatereads it. Retry re-submit()s the stashed input; Dismiss deletes the entry.v2-workspace/layout.tsxis now a reactive 4-way tree: synced row → real workspace UI; unsynced row →WorkspaceCreatingState; no row + failed entry →WorkspaceCreateErrorState; otherwise →WorkspaceNotFoundState.creationStatusenum collapses to anisSyncedboolean (workspaces.$synced),getCreationStatusTextis deleted, and in-flight creates simply render as loading rows.<WorkspaceCreatesManager />is removed from the dashboard layout.Key Decisions
$synced; a separate store is duplicate state.failedWorkspaceCreateslocalStorage collectionnull)WorkspaceCreateRollbackErrorapproach was dropped as over-complex.submit()shape{ workspaceId, completed }awaitthe outcome if they want.New Files
workspace-creates/writeWorkspacePaneLayout.ts— pane-layout write, extracted sosubmit()can run it up-front.CollectionsProvider/workspaceSyncWaits.ts—waitForWorkspaceDeleted()(used by Part 4).Removed Files
workspace-creates/store.ts,workspace-creates/Manager.tsx— the Zustand sidecar.DashboardSidebarWorkspaceItem/utils/getCreationStatusText.ts(+utils/index.ts).Part 3: Shape-stream
onErrorWhy
Electric
ShapeStreams had noonErrorhandler. A single non-retryable error (notably a 401 once the JWT expires) permanently kills the stream — after which everyawaitTxIdon that collection times out. This is the most likely root cause of the txid-timeout reports against #4632.What
handleElectricSyncError(ElectricSyncErrorHandler) wired to all 24 collections'shapeOptions.onError.authClient.token()+setJwt) so the stream can resume authenticated; otherwise it logs. It always returns{}so the stream retries rather than dying.Part 4: Electric-confirmed deletion
Why
The destroy dialog cleared its deleting-state as soon as the host-service teardown returned, before Electric had synced the row's removal — leaving a brief window where a deleted workspace could flash back into the sidebar.
What
useDestroyDialogState.run()nowawaitswaitForWorkspaceDeleted(collections.v2Workspaces, workspaceId)after the destroy saga — it resolves when the row leaves the local collection and rejects after the 30s sync timeout.keepDeletingand surfaces a warning toast instead of clearing state, so the row stays visually deleting rather than reappearing.Manual QA Checklist
Workspace creation
WorkspaceCreateErrorState; Retry re-runs the create; Dismiss removes itWorkspace deletion
Electric sync
Testing
bun run typecheck—@superset/desktop,trpc,host-service,db,electric-proxy✓bun run lint— 0 issues ✓bun test—v2-project(36),task(8),host-service(645), desktopCollectionsProvider(15) ✓Design Decisions
onErrorreturns{}to retry rather than rethrowing — a dead stream is strictly worse than a retrying one; matches the Electric client's intended use ofonError.awaitTxId— observing the row leave the collection avoids retyping all 23 collection slots intoElectricCollectionUtilsjust to callawaitTxIdon the delete path.Known Limitations
parseris configured fortimestamptzcolumns — they sync as ISO strings, notDates. Deferred.$synced/isDeletingguards are intentionally not added to ancillary surfaces (command palette, resource monitor, ports list,/v2-workspaces). The workspace-layout guard covers the real hazard; showing in-flight rows in those lists is consistent with the sidebar.Risks / Rollback
Notes
WIPcommits — squash-merge recommended so the history reads from this description.Summary by cubic
Moves workspace creation to optimistic TanStack DB inserts with Electric txid-confirmed sync, removes the in-flight store, and fixes Electric write-sync correctness and stream resilience. Unsynced workspaces are now non-actionable until they sync, and deletions wait for Electric to confirm before clearing UI state.
New Features
v2Workspacesinsert with a synchronoussubmit()that returns{ workspaceId, completed }; pane layout is written up-front.failedWorkspaceCreateslocal collection; retry re-submits, dismiss clears.isSyncedinstead ofcreationStatus; sidebar renders loading rows for unsynced workspaces; delete waits for the row to leave the local collection and warns on slow sync.onErrorthat retries and refreshes the JWT on 401 across all collections.Bug Fixes
$synced, and sidebar jump shortcuts ignore unsynced or deleting workspaces.pg_current_xact_id()::xid::textwith integer validation; no-op mutations return no txid; routers gate txid and throwNOT_FOUNDon missing rows.awaitTxIdcan match correctly.electric-proxynow usesELECTRIC_PROTOCOL_QUERY_PARAMSfrom@electric-sql/client.Written for commit 3d00569. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Improvements