mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-24 02:39:42 +02:00
fix(workspace): await delete before navigating; suppress self-initiated realtime relocate (MUL-4129) (#4983)
* fix(workspace): drop workspace from list cache while delete is pending (MUL-4129) The delete-workspace flow navigates away before awaiting the DELETE (required ordering — see navigateAwayFromCurrentWorkspace's CancelledError notes), but useDeleteWorkspace left the workspace in the list cache until onSettled. During the pending window any list refetch re-presented the deleting workspace as a selectable/current option. Optimistically remove it in onMutate (after cancelling in-flight list fetches), roll the snapshot back in onError so a failed delete restores the workspace alongside the existing error toast, and keep the onSettled invalidate as the server-truth reconcile. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai> * fix(workspace): own storage cleanup on delete success and tombstone pending deletes (MUL-4129) Address final-review blockers on #4980: 1. The realtime workspace:deleted handler reverse-looks-up the slug from the list cache to clear the ${key}:${slug} persisted namespace; the optimistic removal empties that row on the initiating client, so the lookup misses and cleanup was silently skipped. Capture the slug in onMutate before removal and clear storage in onSuccess only — a failed DELETE rolls back and must not touch persisted state. 2. cancelQueries only covered fetches already in flight at onMutate. Add a pending-delete tombstone (marked onMutate, lifted onSettled before the reconcile invalidate) filtered inside workspaceListOptions' queryFn, so invalidation/reconnect/fetchQuery refetches that land mid-pending cannot write the not-yet-committed row back into cache. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai> * docs(claude-md): scope optimistic updates to same-screen field patches Replace the blanket "mutations optimistic by default" state rule with three scoped rules: optimistic only for predictable same-screen field patches; navigating/confirming flows (create/delete/leave) await the server first; chat send uses the pending-message pattern. Aligned with TanStack Query maintainer guidance and React Router's pending-UI criteria; the old blanket rule is what steered the original MUL-4129 fix toward optimistic entity removal. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(workspace): await delete before navigating; drop optimistic removal (MUL-4129) Rework of the previous approach on this branch. The optimistic removal emptied the workspace list cache at click time, while the settings page was still mounted on the old slug — useWorkspaceId (URL slug + list lookup) then threw 'no workspace selected'. Root cause of the original navigate-first ordering was dual ownership of delete handling between the initiating flow and the realtime workspace:deleted handler. - useDeleteWorkspace: no optimistic removal, no rollback; onMutate only marks the delete self-initiated and captures the slug; onSuccess owns storage cleanup; onSettled invalidates. - pending-delete.ts: repurposed from tombstone filter to self-initiated marker; kept on success (suppresses the WS echo), lifted on failure. - use-realtime-sync: workspace:deleted no-ops for self-initiated deletes; it now only serves deletes initiated elsewhere. - workspace-tab: confirm dialog stays open in loading state, navigate only after the DELETE succeeds; failure leaves the user in place with nothing to roll back. Replace throwing useWorkspaceId with workspace?.id + enabled gating (independent crash on external deletes of the current workspace). Known debt: useLeaveWorkspace still navigates before awaiting (member:removed has no self-initiated marker yet). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
@@ -36,7 +36,9 @@ Keep server state and client state separate.
|
||||
- React Context is for platform plumbing only, such as `WorkspaceIdProvider` and `NavigationProvider`.
|
||||
- Only auth/workspace stores may call `api.*` directly. Other server interaction belongs in queries/mutations.
|
||||
- Workspace-scoped query keys must include `wsId`.
|
||||
- Mutations should be optimistic by default: patch locally, send request, roll back on failure, invalidate on settle.
|
||||
- Optimistic updates only when ALL hold: outcome locally predictable, user stays on the same screen (no navigation), failure is rare, rollback is trivial. Canonical: status/assignee/toggle field patches — patch determinate caches, roll back on failure, invalidate uncertain projections on settle.
|
||||
- Flows that navigate or confirm (create, delete, leave) must await the server before navigating or cleaning up; never optimistically remove an entity from cache.
|
||||
- Chat/message send uses the pending-message pattern: render immediately with a visible pending state and retry on failure, not silent optimism.
|
||||
- WebSocket events invalidate or patch Query cache; they never write directly to Zustand stores.
|
||||
- Persist durable preferences/drafts/layout. Do not persist server data or ephemeral UI state.
|
||||
- Zustand selectors must return stable references. Do not return freshly allocated objects/arrays from selectors without shallow comparison.
|
||||
|
||||
@@ -4,8 +4,14 @@
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import { renderHook } from "@testing-library/react";
|
||||
import type { ReactNode } from "react";
|
||||
import { describe, expect, it, vi, beforeEach } from "vitest";
|
||||
import { describe, expect, it, vi, beforeEach, afterEach } from "vitest";
|
||||
import type { WSClient } from "../api/ws-client";
|
||||
import { defaultStorage } from "../platform/storage";
|
||||
import { workspaceKeys } from "../workspace/queries";
|
||||
import {
|
||||
markWorkspaceDeletePending,
|
||||
unmarkWorkspaceDeletePending,
|
||||
} from "../workspace/pending-delete";
|
||||
import { useRealtimeSync, type RealtimeSyncStores } from "./use-realtime-sync";
|
||||
|
||||
vi.mock("../platform/workspace-storage", () => ({
|
||||
@@ -187,3 +193,59 @@ describe("useRealtimeSync — ws instance change", () => {
|
||||
expect(calls).toContainEqual(["task-messages"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("useRealtimeSync — workspace:deleted self-initiated suppression", () => {
|
||||
let qc: QueryClient;
|
||||
let stores: RealtimeSyncStores;
|
||||
|
||||
beforeEach(() => {
|
||||
qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
||||
stores = createStores();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
unmarkWorkspaceDeletePending("ws-2");
|
||||
localStorage.clear();
|
||||
});
|
||||
|
||||
// getCurrentWsId is mocked to "ws-1" at module level, so deleting "ws-2"
|
||||
// never enters the relocate branch — these tests only exercise the
|
||||
// storage-cleanup path, which is the observable difference between a
|
||||
// handled and a suppressed event.
|
||||
const dispatchWorkspaceDeleted = (ws: WSClient, workspaceId: string) => {
|
||||
const call = vi
|
||||
.mocked(ws.on)
|
||||
.mock.calls.find(([event]) => event === "workspace:deleted");
|
||||
expect(call).toBeDefined();
|
||||
(call![1] as (p: unknown) => void)({ workspace_id: workspaceId });
|
||||
};
|
||||
|
||||
it("ignores the event for a delete this client initiated", () => {
|
||||
const ws = createMockWs();
|
||||
renderHook(() => useRealtimeSync(ws, stores), {
|
||||
wrapper: createWrapper(qc),
|
||||
});
|
||||
qc.setQueryData(workspaceKeys.list(), [{ id: "ws-2", slug: "delete-me" }]);
|
||||
defaultStorage.setItem("multica_issue_draft:delete-me", "draft");
|
||||
|
||||
markWorkspaceDeletePending("ws-2");
|
||||
dispatchWorkspaceDeleted(ws, "ws-2");
|
||||
|
||||
// useDeleteWorkspace.onSuccess owns cleanup for self-initiated deletes;
|
||||
// the handler must not have touched storage.
|
||||
expect(defaultStorage.getItem("multica_issue_draft:delete-me")).toBe("draft");
|
||||
});
|
||||
|
||||
it("still cleans up for a delete initiated elsewhere", () => {
|
||||
const ws = createMockWs();
|
||||
renderHook(() => useRealtimeSync(ws, stores), {
|
||||
wrapper: createWrapper(qc),
|
||||
});
|
||||
qc.setQueryData(workspaceKeys.list(), [{ id: "ws-2", slug: "delete-me" }]);
|
||||
defaultStorage.setItem("multica_issue_draft:delete-me", "draft");
|
||||
|
||||
dispatchWorkspaceDeleted(ws, "ws-2");
|
||||
|
||||
expect(defaultStorage.getItem("multica_issue_draft:delete-me")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -38,6 +38,7 @@ import {
|
||||
notificationPreferenceKeys,
|
||||
} from "../notification-preferences/queries";
|
||||
import { workspaceKeys, workspaceListOptions } from "../workspace/queries";
|
||||
import { isWorkspaceDeletePending } from "../workspace/pending-delete";
|
||||
import {
|
||||
showWebNotification,
|
||||
type SystemNotificationPayload,
|
||||
@@ -794,6 +795,12 @@ export function useRealtimeSync(
|
||||
|
||||
const unsubWsDeleted = ws.on("workspace:deleted", (p) => {
|
||||
const { workspace_id } = p as WorkspaceDeletedPayload;
|
||||
// Self-initiated delete: useDeleteWorkspace owns storage cleanup and
|
||||
// navigation (both run after the DELETE resolves). Reacting here too
|
||||
// would race that flow's navigation with a full-page relocate — the
|
||||
// CancelledError + reload combo this guard exists to prevent. This
|
||||
// handler only serves deletes initiated elsewhere (other user/device).
|
||||
if (isWorkspaceDeletePending(workspace_id)) return;
|
||||
// Event payload has UUID; look up slug from cached workspace list
|
||||
// since clearWorkspaceStorage keys are namespaced by slug.
|
||||
const wsList = qc.getQueryData<{ id: string; slug: string }[]>(workspaceKeys.list()) ?? [];
|
||||
|
||||
177
packages/core/workspace/mutations.test.tsx
Normal file
177
packages/core/workspace/mutations.test.tsx
Normal file
@@ -0,0 +1,177 @@
|
||||
/**
|
||||
* @vitest-environment jsdom
|
||||
*/
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { act, renderHook } from "@testing-library/react";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import type { ReactNode } from "react";
|
||||
import { setApiInstance } from "../api";
|
||||
import type { ApiClient } from "../api/client";
|
||||
import { defaultStorage } from "../platform/storage";
|
||||
import type { Workspace } from "../types";
|
||||
import { useDeleteWorkspace } from "./mutations";
|
||||
import { workspaceKeys } from "./queries";
|
||||
import {
|
||||
isWorkspaceDeletePending,
|
||||
unmarkWorkspaceDeletePending,
|
||||
} from "./pending-delete";
|
||||
|
||||
function createWrapper(qc: QueryClient) {
|
||||
return function Wrapper({ children }: { children: ReactNode }) {
|
||||
return <QueryClientProvider client={qc}>{children}</QueryClientProvider>;
|
||||
};
|
||||
}
|
||||
|
||||
const makeWorkspace = (id: string, slug: string): Workspace => ({
|
||||
id,
|
||||
name: slug,
|
||||
slug,
|
||||
description: null,
|
||||
context: null,
|
||||
settings: {},
|
||||
repos: [],
|
||||
issue_prefix: "MUL",
|
||||
avatar_url: null,
|
||||
created_at: "2026-01-01T00:00:00Z",
|
||||
updated_at: "2026-01-01T00:00:00Z",
|
||||
});
|
||||
|
||||
describe("useDeleteWorkspace", () => {
|
||||
let qc: QueryClient;
|
||||
let deleteWorkspace: ReturnType<typeof vi.fn<(id: string) => Promise<void>>>;
|
||||
let listWorkspaces: ReturnType<typeof vi.fn<() => Promise<Workspace[]>>>;
|
||||
|
||||
const serverList = () => [
|
||||
makeWorkspace("ws-1", "keep-me"),
|
||||
makeWorkspace("ws-2", "delete-me"),
|
||||
];
|
||||
|
||||
const seedList = () => {
|
||||
qc.setQueryData<Workspace[]>(workspaceKeys.list(), serverList());
|
||||
};
|
||||
|
||||
const cachedList = () =>
|
||||
qc.getQueryData<Workspace[]>(workspaceKeys.list()) ?? [];
|
||||
|
||||
beforeEach(() => {
|
||||
qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
||||
deleteWorkspace = vi.fn().mockResolvedValue(undefined);
|
||||
listWorkspaces = vi.fn().mockResolvedValue(serverList());
|
||||
setApiInstance({ deleteWorkspace, listWorkspaces } as unknown as ApiClient);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
qc.clear();
|
||||
// The self-initiated marker is module state and is intentionally KEPT
|
||||
// after a successful delete (it suppresses the WS echo); reset it so
|
||||
// tests stay independent.
|
||||
unmarkWorkspaceDeletePending("ws-2");
|
||||
localStorage.clear();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("leaves the list cache untouched while the DELETE is pending (no optimistic removal)", async () => {
|
||||
seedList();
|
||||
// Hold the DELETE open to observe the pending window. The flow awaits
|
||||
// the mutation with the dialog in a loading state, so the cache must
|
||||
// keep reflecting server truth: the workspace still exists.
|
||||
let resolveDelete!: () => void;
|
||||
deleteWorkspace.mockReturnValue(
|
||||
new Promise<void>((resolve) => {
|
||||
resolveDelete = resolve;
|
||||
}),
|
||||
);
|
||||
|
||||
const { result } = renderHook(() => useDeleteWorkspace(), {
|
||||
wrapper: createWrapper(qc),
|
||||
});
|
||||
|
||||
let mutationDone: Promise<void>;
|
||||
await act(async () => {
|
||||
mutationDone = result.current.mutateAsync("ws-2");
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
expect(deleteWorkspace).toHaveBeenCalledWith("ws-2");
|
||||
expect(cachedList().map((w) => w.id)).toEqual(["ws-1", "ws-2"]);
|
||||
|
||||
await act(async () => {
|
||||
resolveDelete();
|
||||
await mutationDone;
|
||||
});
|
||||
});
|
||||
|
||||
it("invalidates the workspace list after a successful delete", async () => {
|
||||
seedList();
|
||||
const { result } = renderHook(() => useDeleteWorkspace(), {
|
||||
wrapper: createWrapper(qc),
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.mutateAsync("ws-2");
|
||||
});
|
||||
|
||||
expect(qc.getQueryState(workspaceKeys.list())?.isInvalidated).toBe(true);
|
||||
});
|
||||
|
||||
it("clears the deleted slug's workspace-scoped storage on success", async () => {
|
||||
seedList();
|
||||
// The realtime `workspace:deleted` handler skips self-initiated deletes,
|
||||
// so the mutation owns this cleanup; the slug is captured from the list
|
||||
// cache before the mutation fires.
|
||||
defaultStorage.setItem("multica_issue_draft:delete-me", "draft");
|
||||
defaultStorage.setItem("multica_issue_draft:keep-me", "draft");
|
||||
|
||||
const { result } = renderHook(() => useDeleteWorkspace(), {
|
||||
wrapper: createWrapper(qc),
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.mutateAsync("ws-2");
|
||||
});
|
||||
|
||||
expect(defaultStorage.getItem("multica_issue_draft:delete-me")).toBeNull();
|
||||
expect(defaultStorage.getItem("multica_issue_draft:keep-me")).toBe("draft");
|
||||
});
|
||||
|
||||
it("leaves storage and cache untouched when the DELETE fails", async () => {
|
||||
seedList();
|
||||
deleteWorkspace.mockRejectedValue(new Error("boom"));
|
||||
defaultStorage.setItem("multica_issue_draft:delete-me", "draft");
|
||||
|
||||
const { result } = renderHook(() => useDeleteWorkspace(), {
|
||||
wrapper: createWrapper(qc),
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await expect(result.current.mutateAsync("ws-2")).rejects.toThrow("boom");
|
||||
});
|
||||
|
||||
// No optimistic write happened, so there is nothing to roll back.
|
||||
expect(defaultStorage.getItem("multica_issue_draft:delete-me")).toBe("draft");
|
||||
expect(cachedList().map((w) => w.id)).toEqual(["ws-1", "ws-2"]);
|
||||
});
|
||||
|
||||
it("keeps the self-initiated marker after success and lifts it after failure", async () => {
|
||||
seedList();
|
||||
const { result } = renderHook(() => useDeleteWorkspace(), {
|
||||
wrapper: createWrapper(qc),
|
||||
});
|
||||
|
||||
// Success: the id is gone for good; the kept marker suppresses the WS
|
||||
// echo of our own delete whenever it arrives.
|
||||
await act(async () => {
|
||||
await result.current.mutateAsync("ws-2");
|
||||
});
|
||||
expect(isWorkspaceDeletePending("ws-2")).toBe(true);
|
||||
|
||||
// Failure: the workspace still exists, so a later external delete of
|
||||
// the same id must be handled by the realtime handler again.
|
||||
unmarkWorkspaceDeletePending("ws-2");
|
||||
deleteWorkspace.mockRejectedValue(new Error("boom"));
|
||||
await act(async () => {
|
||||
await expect(result.current.mutateAsync("ws-2")).rejects.toThrow("boom");
|
||||
});
|
||||
expect(isWorkspaceDeletePending("ws-2")).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -1,7 +1,13 @@
|
||||
import { useMutation, useQueryClient } from "@tanstack/react-query";
|
||||
import type { Workspace } from "../types";
|
||||
import { api } from "../api";
|
||||
import { defaultStorage } from "../platform/storage";
|
||||
import { clearWorkspaceStorage } from "../platform/storage-cleanup";
|
||||
import { workspaceKeys } from "./queries";
|
||||
import {
|
||||
markWorkspaceDeletePending,
|
||||
unmarkWorkspaceDeletePending,
|
||||
} from "./pending-delete";
|
||||
|
||||
export function useCreateWorkspace() {
|
||||
const qc = useQueryClient();
|
||||
@@ -39,6 +45,34 @@ export function useDeleteWorkspace() {
|
||||
const qc = useQueryClient();
|
||||
return useMutation({
|
||||
mutationFn: (workspaceId: string) => api.deleteWorkspace(workspaceId),
|
||||
// No optimistic removal: the delete flow awaits this mutation with the
|
||||
// confirm dialog in a loading state and only navigates on success, so
|
||||
// the cache staying truthful (server still has the row until commit)
|
||||
// is correct, and a failed DELETE needs no rollback.
|
||||
onMutate: (workspaceId) => {
|
||||
// Mark the delete as self-initiated so the realtime `workspace:deleted`
|
||||
// handler no-ops instead of racing this flow's navigation with its own
|
||||
// full-page relocate. See pending-delete.ts for lifetime rules.
|
||||
markWorkspaceDeletePending(workspaceId);
|
||||
// Capture the slug for onSuccess's storage cleanup — cheap here, and
|
||||
// the row is guaranteed to still be in the list pre-mutation.
|
||||
const slug = qc
|
||||
.getQueryData<Workspace[]>(workspaceKeys.list())
|
||||
?.find((w) => w.id === workspaceId)?.slug;
|
||||
return { slug };
|
||||
},
|
||||
// Success is the only path that clears the deleted workspace's persisted
|
||||
// `${key}:${slug}` namespace — a failed DELETE means the workspace still
|
||||
// exists and its drafts/view state must survive. The realtime handler
|
||||
// skips self-initiated deletes, so cleanup has to happen here.
|
||||
onSuccess: (_data, _workspaceId, ctx) => {
|
||||
if (ctx?.slug) clearWorkspaceStorage(defaultStorage, ctx.slug);
|
||||
},
|
||||
// The workspace still exists after a failed DELETE, so a later external
|
||||
// delete of the same ID must be handled by the realtime handler again.
|
||||
onError: (_err, workspaceId) => {
|
||||
unmarkWorkspaceDeletePending(workspaceId);
|
||||
},
|
||||
onSettled: () => {
|
||||
qc.invalidateQueries({ queryKey: workspaceKeys.list() });
|
||||
},
|
||||
|
||||
32
packages/core/workspace/pending-delete.ts
Normal file
32
packages/core/workspace/pending-delete.ts
Normal file
@@ -0,0 +1,32 @@
|
||||
/**
|
||||
* Registry of workspace IDs whose DELETE was initiated by THIS client.
|
||||
*
|
||||
* Marked in useDeleteWorkspace.onMutate. The realtime `workspace:deleted`
|
||||
* handler checks it and no-ops for self-initiated deletes: the mutation flow
|
||||
* owns storage cleanup and navigation (both run after the DELETE resolves),
|
||||
* and letting the handler react too would race that flow's navigation with
|
||||
* its own full-page relocate.
|
||||
*
|
||||
* Lifted only when the DELETE fails — the workspace still exists, so a later
|
||||
* external delete of the same ID must be handled normally. On success the ID
|
||||
* is gone for good, and keeping the mark suppresses the WS echo of our own
|
||||
* delete no matter when it arrives.
|
||||
*
|
||||
* Module scope rather than React state because the realtime handler runs
|
||||
* outside the component tree. Per-tab by construction: other tabs/devices
|
||||
* have an empty registry, so their handlers process the event normally.
|
||||
*/
|
||||
const pendingDeletes = new Set<string>();
|
||||
|
||||
export function markWorkspaceDeletePending(workspaceId: string) {
|
||||
pendingDeletes.add(workspaceId);
|
||||
}
|
||||
|
||||
export function unmarkWorkspaceDeletePending(workspaceId: string) {
|
||||
pendingDeletes.delete(workspaceId);
|
||||
}
|
||||
|
||||
/** True if this client initiated a DELETE for the workspace. */
|
||||
export function isWorkspaceDeletePending(workspaceId: string): boolean {
|
||||
return pendingDeletes.has(workspaceId);
|
||||
}
|
||||
@@ -21,7 +21,6 @@ import { toast } from "sonner";
|
||||
import { useQuery, useQueryClient } from "@tanstack/react-query";
|
||||
import { useAuthStore } from "@multica/core/auth";
|
||||
import { useLeaveWorkspace, useDeleteWorkspace } from "@multica/core/workspace/mutations";
|
||||
import { useWorkspaceId } from "@multica/core/hooks";
|
||||
import {
|
||||
memberListOptions,
|
||||
workspaceKeys,
|
||||
@@ -46,8 +45,17 @@ export function WorkspaceTab() {
|
||||
const { t } = useT("settings");
|
||||
const user = useAuthStore((s) => s.user);
|
||||
const workspace = useCurrentWorkspace();
|
||||
const wsId = useWorkspaceId();
|
||||
const { data: members = [], isFetched: membersFetched } = useQuery(memberListOptions(wsId));
|
||||
// Derive the id from useCurrentWorkspace instead of the throwing
|
||||
// useWorkspaceId: this component can legitimately render while the
|
||||
// workspace is gone from the list cache but the URL slug hasn't changed
|
||||
// yet (post-delete invalidation before navigation completes, or an
|
||||
// external delete of the workspace we're on). The `!workspace` guard
|
||||
// below renders null for that window; a throwing hook would crash first.
|
||||
const wsId = workspace?.id;
|
||||
const { data: members = [], isFetched: membersFetched } = useQuery({
|
||||
...memberListOptions(wsId ?? ""),
|
||||
enabled: !!wsId,
|
||||
});
|
||||
const qc = useQueryClient();
|
||||
const leaveWorkspace = useLeaveWorkspace();
|
||||
const deleteWorkspace = useDeleteWorkspace();
|
||||
@@ -55,36 +63,34 @@ export function WorkspaceTab() {
|
||||
const hasOnboarded = useHasOnboarded();
|
||||
|
||||
/**
|
||||
* Send the user to a safe URL BEFORE the leave/delete mutation fires.
|
||||
* The destination is computed from the current cached workspace list,
|
||||
* minus the workspace that's about to go away.
|
||||
* Send the user to a safe URL, computed from the current cached workspace
|
||||
* list minus the workspace that's going away.
|
||||
*
|
||||
* Why navigate first, not after:
|
||||
* 1. The backend broadcasts `workspace:deleted` / `member:removed` the
|
||||
* moment the mutation lands. If the user is still on the soon-to-
|
||||
* be-deleted workspace's URL when that event arrives, the realtime
|
||||
* handler in `use-realtime-sync.ts` also triggers a relocation —
|
||||
* and both code paths race with the mutation's own
|
||||
* `invalidateQueries` refetch. The loser's in-flight fetch gets
|
||||
* cancelled, surfacing as an unhandled `CancelledError`.
|
||||
* 2. Navigating first means by the time the WS event fires, the
|
||||
* active workspace is already something else; the realtime
|
||||
* handler's "current === deleted" check fails and its relocate
|
||||
* branch no-ops.
|
||||
* 3. UX: the destructive flow feels instant (dialog closes → new
|
||||
* workspace appears) even though the API hasn't responded yet.
|
||||
* Call ordering differs per flow:
|
||||
* - Delete calls this AFTER the mutation succeeds. The realtime
|
||||
* `workspace:deleted` handler skips self-initiated deletes (see
|
||||
* pending-delete.ts), so nothing races this navigation.
|
||||
* - Leave still calls this BEFORE the mutation fires: `member:removed`
|
||||
* has no self-initiated marker yet, so if the user were still on the
|
||||
* workspace's URL when that event arrives, the realtime handler in
|
||||
* `use-realtime-sync.ts` would trigger a parallel full-page relocate
|
||||
* that races the mutation's `invalidateQueries` refetch — the loser's
|
||||
* in-flight fetch gets cancelled, surfacing as an unhandled
|
||||
* `CancelledError`. Navigating first makes the handler's
|
||||
* "current === lost workspace" check fail and its relocate no-op.
|
||||
* Known debt: give leave the same await-then-navigate shape as delete.
|
||||
*/
|
||||
const navigateAwayFromCurrentWorkspace = () => {
|
||||
const cachedList =
|
||||
qc.getQueryData<Workspace[]>(workspaceListOptions().queryKey) ?? [];
|
||||
const remaining = cachedList.filter((w) => w.id !== workspace?.id);
|
||||
// Clear the workspace-context singleton BEFORE navigating and BEFORE
|
||||
// the mutation fires. Three downstream consumers read it:
|
||||
// 1. Realtime `workspace:deleted` handler's "current === deleted"
|
||||
// check — if the singleton still points at the deleting workspace
|
||||
// when the WS event arrives, it fires a parallel relocate that
|
||||
// races the mutation's invalidate and the settings page's own
|
||||
// navigate, surfacing a CancelledError and a full-page reload.
|
||||
// Clear the workspace-context singleton BEFORE navigating. Three
|
||||
// downstream consumers read it:
|
||||
// 1. Realtime relocate handlers' "current === lost workspace" check
|
||||
// (`member:removed` for leave; also a second line of defense for
|
||||
// delete) — if the singleton still points at the lost workspace
|
||||
// when the WS event arrives, they fire a parallel full-page
|
||||
// relocate that races this navigation.
|
||||
// 2. Chrome gating (`{slug && <AppSidebar />}` on desktop) — if the
|
||||
// singleton lingers, the sidebar stays mounted while the deleted
|
||||
// workspace is no longer in the list, and `useWorkspaceId` throws.
|
||||
@@ -237,14 +243,17 @@ export function WorkspaceTab() {
|
||||
const handleConfirmDelete = async () => {
|
||||
if (!workspace) return;
|
||||
setActionId("delete-workspace");
|
||||
// Close the dialog and navigate away FIRST. See navigateAwayFromCurrentWorkspace
|
||||
// comment for why: keeps the realtime `workspace:deleted` handler out
|
||||
// of the race so we don't end up with concurrent refetches cancelling
|
||||
// each other and surfacing CancelledError.
|
||||
setDeleteDialogOpen(false);
|
||||
navigateAwayFromCurrentWorkspace();
|
||||
// Await the DELETE with the dialog in its loading state, and only
|
||||
// navigate on success (CLAUDE.md: flows that navigate must await the
|
||||
// server; no optimistic removal). The realtime `workspace:deleted`
|
||||
// handler skips self-initiated deletes via the pending-delete registry,
|
||||
// so it can't race this navigation with its own full-page relocate.
|
||||
// On failure the dialog stays open, the cache was never touched, and
|
||||
// the user is exactly where they started.
|
||||
try {
|
||||
await deleteWorkspace.mutateAsync(workspace.id);
|
||||
setDeleteDialogOpen(false);
|
||||
navigateAwayFromCurrentWorkspace();
|
||||
} catch (e) {
|
||||
toast.error(e instanceof Error ? e.message : t(($) => $.workspace.toast_delete_failed));
|
||||
} finally {
|
||||
|
||||
Reference in New Issue
Block a user