[codex] Render cached workspaces before sync#4820
Conversation
|
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 4 individual chapters for you:
Chapters generated by Stage for commit fa650ea on May 21, 2026 9:50pm UTC. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR removes ChangesWorkspace Host Readiness Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
✨ 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 |
Greptile SummaryThis PR removes Electric sync-state gates (
Confidence Score: 4/5Safe to merge; the only observable regression is a brief flash of workspace content for incompatible remote hosts before the incompatibility screen takes over. All changes are intentional and the logic holds for the common path. The one edge to watch is useRemoteHostStatus.ts — the immediate "ready" return before infoQuery settles is the only behavior worth validating against incompatible-host scenarios.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsx | Removes canUseWorkspace guard; workspace now renders from cache without waiting for $synced, and ensureWorkspaceInSidebar fires as soon as workspace row is non-null. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/hooks/useRemoteHostStatus/useRemoteHostStatus.ts | Drops isReady from the hosts live-query; for remote workspaces the function now falls through to "ready" immediately, before infoQuery resolves, which can briefly show workspace content before an incompatible-host screen appears. |
| apps/desktop/src/renderer/hooks/host-service/useWorkspaceHostUrl/useWorkspaceHostUrl.ts | Removes isSynced selection and the resulting loading gate; the existing isReady && !match guard still covers the true empty-cache case. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarData/useDashboardSidebarData.ts | Removes isSynced from the mapped workspace object, consistent with type change; no logic altered. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/types.ts | Removes isSynced from DashboardSidebarWorkspace interface, aligning the type with the mapping change. |
Sequence Diagram
sequenceDiagram
participant Cache as TanStack DB Cache
participant Layout as V2WorkspaceLayout
participant RHS as useRemoteHostStatus
participant IQ as infoQuery (React Query)
participant Electric as Electric Sync
Note over Cache,Electric: PR change: render from cache without waiting for Electric $synced
Cache->>Layout: "workspace row (cached, $synced=false)"
Layout->>RHS: useRemoteHostStatus(workspace)
RHS-->>Layout: "status=ready (immediate, no isReady gate)"
Layout->>Layout: render WorkspaceProvider + Outlet
IQ-->>RHS: version check resolves
alt host incompatible
RHS-->>Layout: "status=incompatible"
Layout->>Layout: render WorkspaceHostIncompatibleState
end
Electric-->>Cache: "$synced=true"
Cache->>Layout: workspace row updated
Layout->>Layout: clearWorkspaceTransaction (insert done)
Comments Outside Diff (1)
-
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/hooks/useRemoteHostStatus/useRemoteHostStatus.ts, line 68-83 (link)Workspace may briefly flash before incompatibility is shown
After removing the
isReady && !hostRowguard, the function falls straight through to{ status: "ready" }for any remote workspace whileinfoQueryis still in-flight.layout.tsxrenders<WorkspaceProvider>/<Outlet>for any "ready" status, so ifinfoQuerylater resolves to an incompatible version, the user sees a flash of live workspace content before theWorkspaceHostIncompatibleStatereplaces it. Previously, theisReadygate provided at least a brief loading state that masked this race. The impact is narrow (only incompatible hosts) and may be an acceptable UX trade-off, but it is worth noting sincehostStatus.status === "loading"inlayout.tsxno longer fires for remote workspaces at all.Prompt To Fix With AI
This is a comment left during a code review. Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/hooks/useRemoteHostStatus/useRemoteHostStatus.ts Line: 68-83 Comment: **Workspace may briefly flash before incompatibility is shown** After removing the `isReady && !hostRow` guard, the function falls straight through to `{ status: "ready" }` for any remote workspace while `infoQuery` is still in-flight. `layout.tsx` renders `<WorkspaceProvider>` / `<Outlet>` for any "ready" status, so if `infoQuery` later resolves to an incompatible version, the user sees a flash of live workspace content before the `WorkspaceHostIncompatibleState` replaces it. Previously, the `isReady` gate provided at least a brief loading state that masked this race. The impact is narrow (only incompatible hosts) and may be an acceptable UX trade-off, but it is worth noting since `hostStatus.status === "loading"` in `layout.tsx` no longer fires for remote workspaces at all. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/hooks/useRemoteHostStatus/useRemoteHostStatus.ts:68-83
**Workspace may briefly flash before incompatibility is shown**
After removing the `isReady && !hostRow` guard, the function falls straight through to `{ status: "ready" }` for any remote workspace while `infoQuery` is still in-flight. `layout.tsx` renders `<WorkspaceProvider>` / `<Outlet>` for any "ready" status, so if `infoQuery` later resolves to an incompatible version, the user sees a flash of live workspace content before the `WorkspaceHostIncompatibleState` replaces it. Previously, the `isReady` gate provided at least a brief loading state that masked this race. The impact is narrow (only incompatible hosts) and may be an acceptable UX trade-off, but it is worth noting since `hostStatus.status === "loading"` in `layout.tsx` no longer fires for remote workspaces at all.
Reviews (1): Last reviewed commit: "Render cached workspaces before sync" | Re-trigger Greptile
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
1 participant
Summary
$syncedonce a cached workspace row exists.isReady.isSyncedfield from the rendered workspace model so components cannot accidentally block on it.Root Cause
TanStack DB live queries can return persisted rows before the Electric collection reports readiness or
$synced === true. The v2 workspace route and host-target hook were still treating those sync states as UI readiness, so an Electric outage or delayed shape could blank an otherwise cached workspace.Impact
Cached workspace rows now remain usable while Electric catches up.
isReadyis only used to distinguish loading from not-found when no row exists, and$syncedremains only as internal bookkeeping for clearing pending insert state.Validation
bun run lint:fixbun run lintbun run --cwd apps/desktop typecheckSummary by cubic
Render cached workspace rows before Electric sync so the v2 workspace layout and remote host status don’t blank while shapes catch up. Improves resiliency during Electric outages or delays.
$synced/isReadywhen a cached workspace row exists.useRemoteHostStatus.isSyncedfrom the sidebar workspace model and related gating inV2WorkspaceLayout.Written for commit fa650ea. Summary will update on new commits. Review in cubic
Summary by CodeRabbit