Skip to content

fix(desktop): gate v2 workspace children on collection readiness #3464

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
saddlepaddle merged 1 commit into from
Apr 15, 2026
+5 −13
Merged

fix(desktop): gate v2 workspace children on collection readiness #3464

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 5 additions & 13 deletions apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ function V2WorkspaceLayout() {
const { machineId, activeHostUrl } = useLocalHostService();
const { ensureWorkspaceInSidebar } = useDashboardSidebarState();

const { data: workspacesWithHost = [] } = useLiveQuery(
const { data: workspacesWithHost = [], isReady } = useLiveQuery(
(q) =>
q
.from({ v2Workspaces: collections.v2Workspaces })
Expand Down Expand Up @@ -64,22 +64,14 @@ function V2WorkspaceLayout() {
ensureWorkspaceInSidebar(workspace.id, workspace.projectId);
}, [ensureWorkspaceInSidebar, workspace]);

// TODO: This renders child routes without WorkspaceTrpcProvider when
// the workspace hasn't loaded from collections yet, or during route
// transitions (e.g. navigating away from a workspace). If the outgoing
// workspace page hasn't fully unmounted, its components (TerminalPane,
// etc.) will crash with "useWorkspaceClient must be used within
// WorkspaceClientProvider". Either the layout should never render
// children without the provider, or the provider should move to the
// page level so each page owns its own context.
if (!workspaceId || !workspace) {
return <Outlet />;
if (!workspaceId || !isReady) {
return null;
}
Comment on lines +67 to 69
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 No loading indicator during collection hydration

Returning null while !isReady eliminates the crash, but it also produces a blank screen during the initial load and every workspace transition. Depending on how long isReady takes to flip, users may see a momentary flash of nothing. A lightweight loading state (spinner, skeleton, or even a static placeholder) would avoid that:

Suggested change
if (!workspaceId || !isReady) {
return null;
}
if (!workspaceId || !isReady) {
return (
<div className="flex h-full w-full items-center justify-center text-muted-foreground">
Loading workspace…
</div>
);
}

This is a non-blocking suggestion; the crash fix is the important part.


if (!hostUrl) {
if (!workspace || !hostUrl) {
return (
<div className="flex h-full w-full items-center justify-center text-muted-foreground">
Workspace host service not available
Workspace not found
</div>
Comment on lines +71 to 75
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Keep the missing-workspace and missing-host states separate.

Line 71 now maps both !workspace and !hostUrl to "Workspace not found". That makes a valid workspace with an unavailable or still-bootstrapping local host look like a bad ID. Please keep the new not-found state for !workspace, but preserve a distinct fallback for !hostUrl.

🔧 Suggested split 
-if (!workspace || !hostUrl) {
+if (!workspace) {
   return (
     <div className="flex h-full w-full items-center justify-center text-muted-foreground">
       Workspace not found
     </div>
   );
 }
+
+if (!hostUrl) {
+  return (
+    <div className="flex h-full w-full items-center justify-center text-muted-foreground">
+      Workspace host service not available
+    </div>
+  );
+}
📝 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
if (!workspace || !hostUrl) {
return (
<div className="flex h-full w-full items-center justify-center text-muted-foreground">
Workspace host service not available
Workspace not found
</div>
if (!workspace) {
return (
<div className="flex h-full w-full items-center justify-center text-muted-foreground">
Workspace not found
</div>
);
}
if (!hostUrl) {
return (
<div className="flex h-full w-full items-center justify-center text-muted-foreground">
Workspace host service not available
</div>
);
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsx`
around lines 71 - 75, The current conditional lumps both missing workspace and
missing host into the same message; update the JSX in the layout component so
that the check for workspace and hostUrl are separate: when !workspace return
the existing "Workspace not found" fallback, but when workspace exists and
!hostUrl return a distinct fallback (e.g., "Host unavailable" or "Waiting for
host") so a valid workspace isn't mistaken for a bad ID; adjust the return logic
in the component that references the workspace and hostUrl variables
accordingly.

);
Comment on lines +71 to 76
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Misleading error message when host service is unavailable

The two sub-cases covered by !workspace || !hostUrl have meaningfully different root causes:

  1. !workspace — the id doesn't resolve in the collection → "Workspace not found" is correct.
  2. !hostUrl while workspace != null — this only happens for a local workspace when activeHostUrl is null (i.e., the host service hasn't started or has lost its connection). In that scenario the workspace does exist, but showing "Workspace not found" is misleading.

Before this PR the two cases had distinct messages ("Workspace host service not available" vs. silent <Outlet />). Splitting them again makes the error actionable:

Suggested change
if (!workspace || !hostUrl) {
return (
<div className="flex h-full w-full items-center justify-center text-muted-foreground">
Workspace host service not available
Workspace not found
</div>
);
if (!workspace) {
return (
<div className="flex h-full w-full items-center justify-center text-muted-foreground">
Workspace not found
</div>
);
}
if (!hostUrl) {
return (
<div className="flex h-full w-full items-center justify-center text-muted-foreground">
Workspace host service not available
</div>
);
}

Comment on lines +71 to 76
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Apr 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This condition conflates two distinct error states: !workspace (ID doesn't resolve) and !hostUrl (host service unavailable while workspace exists). Showing "Workspace not found" when the host service is down is misleading — the workspace does exist. Split into separate checks to preserve the actionable "Workspace host service not available" message that existed before this change.

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 71:

<comment>This condition conflates two distinct error states: `!workspace` (ID doesn't resolve) and `!hostUrl` (host service unavailable while workspace exists). Showing "Workspace not found" when the host service is down is misleading — the workspace does exist. Split into separate checks to preserve the actionable "Workspace host service not available" message that existed before this change.</comment>

<file context>
@@ -64,22 +64,14 @@ function V2WorkspaceLayout() {
 	}
 
-	if (!hostUrl) {
+	if (!workspace || !hostUrl) {
 		return (
 			<div className="flex h-full w-full items-center justify-center text-muted-foreground">
</file context>
Suggested change
if (!workspace || !hostUrl) {
return (
<div className="flex h-full w-full items-center justify-center text-muted-foreground">
Workspace host service not available
Workspace not found
</div>
);
if (!workspace) {
return (
<div className="flex h-full w-full items-center justify-center text-muted-foreground">
Workspace not found
</div>
);
}
if (!hostUrl) {
return (
<div className="flex h-full w-full items-center justify-center text-muted-foreground">
Workspace host service not available
</div>
);
}
Fix with Cubic

}
Expand Down
Loading