Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export function useWorkspaceHostTarget(
.select(({ workspaces }) => ({
organizationId: workspaces.organizationId,
hostId: workspaces.hostId,
isSynced: workspaces.$synced,
})),
[collections, workspaceId],
);
Expand All @@ -43,6 +44,7 @@ export function useWorkspaceHostTarget(
return useMemo(() => {
if (!workspaceId || !isReady) return { status: "loading" };
if (!match) return { status: "not-found" };
if (!match.isSynced) return { status: "loading" };
if (machineId && match.hostId === machineId) {
if (activeHostUrl) {
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {
} from "renderer/hooks/host-service/useDestroyWorkspace";
import { useV2UserPreferences } from "renderer/hooks/useV2UserPreferences/useV2UserPreferences";
import { useNavigateAwayFromWorkspace } from "renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useNavigateAwayFromWorkspace";
import { useCollections } from "renderer/routes/_authenticated/providers/CollectionsProvider";
import { waitForWorkspaceDeleted } from "renderer/routes/_authenticated/providers/CollectionsProvider/workspaceSyncWaits";
import { useDeletingWorkspaces } from "renderer/routes/_authenticated/providers/DeletingWorkspacesProvider";

interface UseDestroyDialogStateOptions {
Expand All @@ -34,6 +36,7 @@ export function useDestroyDialogState({
onDeleted,
}: UseDestroyDialogStateOptions) {
const { destroy, inspect, hostTarget } = useDestroyWorkspace(workspaceId);
const collections = useCollections();
const { markDeleting, clearDeleting } = useDeletingWorkspaces();
const { navigateAwayFromWorkspace } = useNavigateAwayFromWorkspace();

Expand Down Expand Up @@ -109,6 +112,7 @@ export function useDestroyDialogState({
async (force: boolean) => {
if (inFlight.current) return;
inFlight.current = true;
let keepDeleting = false;

setError(null);
onOpenChange(false);
Expand Down Expand Up @@ -136,6 +140,20 @@ export function useDestroyDialogState({
throw firstErr;
}
}
try {
await waitForWorkspaceDeleted(collections.v2Workspaces, workspaceId);
} catch (syncErr) {
keepDeleting = true;
onDeleted?.();
console.warn("[workspace-delete] delete synced slowly", {
workspaceId,
err: syncErr,
});
toast.warning(
`Deleted ${workspaceName}, but sync is taking longer than expected.`,
);
return;
}
for (const warning of result.warnings) toast.warning(warning);
onDeleted?.();
} catch (err) {
Expand All @@ -151,7 +169,9 @@ export function useDestroyDialogState({
);
}
} finally {
clearDeleting(workspaceId);
if (!keepDeleting) {
clearDeleting(workspaceId);
}
inFlight.current = false;
}
Comment on lines 143 to 176
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 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.

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.

},
Expand All @@ -165,6 +185,7 @@ export function useDestroyDialogState({
markDeleting,
clearDeleting,
navigateAwayFromWorkspace,
collections,
],
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { useOptimisticCollectionActions } from "renderer/routes/_authenticated/h
import { useDeletingWorkspaces } from "renderer/routes/_authenticated/providers/DeletingWorkspacesProvider";
import { RenameBranchDialog } from "renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/components";
import { useV2WorkspaceNotificationStatus } from "renderer/stores/v2-notifications";
import { useWorkspaceCreatesStore } from "renderer/stores/workspace-creates";
import { useDashboardSidebarHover } from "../../providers/DashboardSidebarHoverProvider";
import type { DashboardSidebarWorkspace } from "../../types";
import { DashboardSidebarDeleteDialog } from "../DashboardSidebarDeleteDialog";
Expand Down Expand Up @@ -36,7 +35,7 @@ export function DashboardSidebarWorkspaceItem({
hostIsOnline,
name,
branch,
creationStatus,
isSynced,
pullRequest,
} = workspace;
const isMainWorkspace = workspace.type === "main";
Expand Down Expand Up @@ -77,16 +76,11 @@ export function DashboardSidebarWorkspaceItem({
const handleAfterBranchRename = (newBranchName: string) => {
v2WorkspaceActions.updateWorkspace(id, { branch: newBranchName });
};
const isPending = !!creationStatus;
const isFailedInFlight = creationStatus === "failed";
const isPending = !isSynced;
// Keep the delete dialog outside the hidden wrapper below — the destroy
// flow reopens it into an error pane on conflict/teardown-failed.
const isDeleting = useDeletingWorkspaces().isDeleting(id);

const handleDismissInFlight = useCallback(() => {
useWorkspaceCreatesStore.getState().remove(id);
}, [id]);

const {
hoveredId: hoverHoveredId,
requestOpen: hoverRequestOpen,
Expand Down Expand Up @@ -142,11 +136,9 @@ export function DashboardSidebarWorkspaceItem({
isActive={isActive}
workspaceStatus={workspaceStatus}
onClick={handleClick}
creationStatus={creationStatus}
isSynced={isSynced}
pullRequestState={pullRequest?.state ?? null}
aria-label={
creationStatus ? `Creating workspace: ${name}` : undefined
}
aria-label={isPending ? `Creating workspace: ${name}` : undefined}
/>
</div>
);
Expand Down Expand Up @@ -226,11 +218,7 @@ export function DashboardSidebarWorkspaceItem({
onClick={handleClick}
onDoubleClick={isPending ? undefined : startRename}
onRemoveFromSidebarClick={handleRemoveFromSidebar}
onCloseWorkspaceClick={
isFailedInFlight
? handleDismissInFlight
: () => setIsDeleteDialogOpen(true)
}
onCloseWorkspaceClick={() => setIsDeleteDialogOpen(true)}
onRenameValueChange={setRenameValue}
onSubmitRename={submitRename}
onCancelRename={cancelRename}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ interface DashboardSidebarCollapsedWorkspaceButtonProps
hostIsOnline: boolean | null;
isActive: boolean;
workspaceStatus?: ActivePaneStatus | null;
creationStatus?: "preparing" | "generating-branch" | "creating" | "failed";
isSynced: boolean;
pullRequestState?: DashboardSidebarWorkspacePullRequest["state"] | null;
}

Expand All @@ -30,7 +30,7 @@ export const DashboardSidebarCollapsedWorkspaceButton = forwardRef<
hostIsOnline,
isActive,
workspaceStatus = null,
creationStatus,
isSynced,
pullRequestState = null,
className,
...props
Expand All @@ -56,7 +56,7 @@ export const DashboardSidebarCollapsedWorkspaceButton = forwardRef<
isActive={isActive}
variant="collapsed"
workspaceStatus={workspaceStatus}
creationStatus={creationStatus}
isSynced={isSynced}
pullRequestState={pullRequestState}
/>
</button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
type ComponentPropsWithoutRef,
forwardRef,
useEffect,
useMemo,
useRef,
} from "react";
import { HiMiniMinus, HiMiniXMark } from "react-icons/hi2";
Expand All @@ -17,7 +16,6 @@ import type {
DashboardSidebarWorkspace,
DashboardSidebarWorkspacePullRequest,
} from "../../../../types";
import { getCreationStatusText } from "../../utils/getCreationStatusText";
import { DashboardSidebarWorkspaceDiffStats } from "../DashboardSidebarWorkspaceDiffStats";
import { DashboardSidebarWorkspaceIcon } from "../DashboardSidebarWorkspaceIcon";

Expand Down Expand Up @@ -83,8 +81,9 @@ export const DashboardSidebarExpandedWorkspaceRow = forwardRef<
name,
branch,
pullRequest,
creationStatus,
isSynced,
} = workspace;
const isPending = !isSynced;
const showsStandaloneActiveStripe = accentColor == null;
const localRef = useRef<HTMLDivElement>(null);
const openUrl = electronTrpc.external.openUrl.useMutation();
Expand All @@ -98,10 +97,7 @@ export const DashboardSidebarExpandedWorkspaceRow = forwardRef<
}
}, [isActive]);

const creationStatusText = useMemo(
() => getCreationStatusText(creationStatus),
[creationStatus],
);
const creationStatusText = isPending ? "Creating…" : null;
const isMainWorkspace = workspace.type === "main";
const workspaceKindTitle = isMainWorkspace
? "Main workspace"
Expand All @@ -115,7 +111,7 @@ export const DashboardSidebarExpandedWorkspaceRow = forwardRef<
<div
role={onClick ? "button" : undefined}
tabIndex={onClick ? 0 : undefined}
aria-disabled={creationStatus ? true : undefined}
aria-disabled={isPending ? true : undefined}
ref={(node) => {
localRef.current = node;
if (typeof ref === "function") ref(node);
Expand Down Expand Up @@ -174,7 +170,7 @@ export const DashboardSidebarExpandedWorkspaceRow = forwardRef<
isActive={isActive}
variant="expanded"
workspaceStatus={workspaceStatus}
creationStatus={creationStatus}
isSynced={isSynced}
pullRequestState={pullRequest.state}
/>
</button>
Expand All @@ -187,7 +183,7 @@ export const DashboardSidebarExpandedWorkspaceRow = forwardRef<
isActive={isActive}
variant="expanded"
workspaceStatus={workspaceStatus}
creationStatus={creationStatus}
isSynced={isSynced}
pullRequestState={null}
/>
</div>
Expand Down Expand Up @@ -256,14 +252,7 @@ export const DashboardSidebarExpandedWorkspaceRow = forwardRef<

<div className="col-start-2 row-start-1 grid h-5 shrink-0 items-center justify-items-end [&>*]:col-start-1 [&>*]:row-start-1">
{creationStatusText ? (
<span
className={cn(
"text-[11px]",
creationStatus === "failed"
? "text-destructive group-hover:hidden"
: "text-muted-foreground",
)}
>
<span className="text-[11px] text-muted-foreground">
{creationStatusText}
</span>
) : (
Expand All @@ -276,9 +265,9 @@ export const DashboardSidebarExpandedWorkspaceRow = forwardRef<
/>
)
)}
{(!creationStatus || creationStatus === "failed") && (
{isSynced && (
<div className="hidden items-center justify-end gap-1.5 group-hover:flex">
{shortcutLabel && !creationStatus && (
{shortcutLabel && (
<span className="shrink-0 font-mono text-[10px] tabular-nums text-muted-foreground">
{shortcutLabel}
</span>
Expand Down Expand Up @@ -330,24 +319,16 @@ export const DashboardSidebarExpandedWorkspaceRow = forwardRef<
}
}}
className="flex items-center justify-center text-muted-foreground hover:text-foreground"
aria-label={
creationStatus === "failed"
? "Dismiss"
: "Close workspace"
}
aria-label="Close workspace"
>
<HiMiniXMark className="size-3.5" />
</button>
</TooltipTrigger>
<TooltipContent side="top" sideOffset={4}>
{creationStatus === "failed" ? (
"Dismiss"
) : (
<HotkeyLabel
label="Close workspace"
id={isActive ? "CLOSE_WORKSPACE" : undefined}
/>
)}
<HotkeyLabel
label="Close workspace"
id={isActive ? "CLOSE_WORKSPACE" : undefined}
/>
</TooltipContent>
</Tooltip>
)}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { cn } from "@superset/ui/utils";
import { CgLaptop } from "react-icons/cg";
import { HiExclamationTriangle } from "react-icons/hi2";
import {
LuGitMerge,
LuGitPullRequest,
Expand All @@ -25,7 +24,7 @@ interface DashboardSidebarWorkspaceIconProps {
isActive: boolean;
variant: "collapsed" | "expanded";
workspaceStatus?: ActivePaneStatus | null;
creationStatus?: "preparing" | "generating-branch" | "creating" | "failed";
isSynced: boolean;
pullRequestState?: DashboardSidebarWorkspacePullRequest["state"] | null;
}

Expand Down Expand Up @@ -55,7 +54,7 @@ export function DashboardSidebarWorkspaceIcon({
isActive,
variant,
workspaceStatus = null,
creationStatus,
isSynced,
pullRequestState = null,
}: DashboardSidebarWorkspaceIconProps) {
const overlayPosition = OVERLAY_POSITION[variant];
Expand Down Expand Up @@ -103,9 +102,7 @@ export function DashboardSidebarWorkspaceIcon({

return (
<>
{creationStatus === "failed" ? (
<HiExclamationTriangle className="size-4 text-destructive" />
) : creationStatus || workspaceStatus === "working" ? (
{!isSynced || workspaceStatus === "working" ? (
<AsciiSpinner className="text-base" />
) : (
renderPrimaryIcon()
Expand Down

This file was deleted.

This file was deleted.

Loading
Loading