fix: clear kanban snapshots when active workspace changes (#633)
This commit is contained in:
@@ -41,12 +41,16 @@ function WorkspaceSection({
|
||||
value={activeWorkspaceId ?? ""}
|
||||
onValueChange={(value) => onWorkspaceChange(value || null)}
|
||||
>
|
||||
<SelectTrigger className="w-full border-border">
|
||||
<SelectTrigger className="w-full border-border" data-testid="workspace-select-trigger">
|
||||
<SelectValue placeholder="Select workspace" />
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
{workspaces.map((workspace: Workspace) => (
|
||||
<SelectItem key={workspace.id} value={workspace.id}>
|
||||
<SelectItem
|
||||
key={workspace.id}
|
||||
value={workspace.id}
|
||||
data-testid={`workspace-select-item-${workspace.id}`}
|
||||
>
|
||||
{workspace.name}
|
||||
</SelectItem>
|
||||
))}
|
||||
|
||||
@@ -0,0 +1,82 @@
|
||||
// Switching the active workspace must not leak tasks from the previous workspace into the sidebar.
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import { execSync } from "node:child_process";
|
||||
import { test, expect } from "../../fixtures/test-base";
|
||||
import { KanbanPage } from "../../pages/kanban-page";
|
||||
import { SessionPage } from "../../pages/session-page";
|
||||
|
||||
test.describe("Sidebar — cross-workspace isolation", () => {
|
||||
test("tasks from the previous workspace do not leak into the sidebar after switching", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
backend,
|
||||
}) => {
|
||||
// --- Seed workspace A artifacts ---
|
||||
const taskA = await apiClient.createTask(seedData.workspaceId, "Workspace A Task", {
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
});
|
||||
|
||||
// --- Create workspace B with its own workflow, repo, and task ---
|
||||
const workspaceB = await apiClient.createWorkspace("Workspace B");
|
||||
const workflowB = await apiClient.createWorkflow(workspaceB.id, "Workflow B", "simple");
|
||||
const { steps: stepsB } = await apiClient.listWorkflowSteps(workflowB.id);
|
||||
const startStepB = [...stepsB]
|
||||
.sort((a, b) => a.position - b.position)
|
||||
.find((s) => s.is_start_step);
|
||||
if (!startStepB) throw new Error("workspace B workflow has no start step");
|
||||
|
||||
const repoBDir = path.join(backend.tmpDir, "repos", "e2e-repo-b");
|
||||
fs.mkdirSync(repoBDir, { recursive: true });
|
||||
const gitEnv = {
|
||||
...process.env,
|
||||
HOME: backend.tmpDir,
|
||||
GIT_AUTHOR_NAME: "E2E Test",
|
||||
GIT_AUTHOR_EMAIL: "e2e@test.local",
|
||||
GIT_COMMITTER_NAME: "E2E Test",
|
||||
GIT_COMMITTER_EMAIL: "e2e@test.local",
|
||||
};
|
||||
execSync("git init -b main", { cwd: repoBDir, env: gitEnv });
|
||||
execSync('git commit --allow-empty -m "init"', { cwd: repoBDir, env: gitEnv });
|
||||
const repoB = await apiClient.createRepository(workspaceB.id, repoBDir);
|
||||
|
||||
const taskB = await apiClient.createTask(workspaceB.id, "Workspace B Task", {
|
||||
workflow_id: workflowB.id,
|
||||
workflow_step_id: startStepB.id,
|
||||
repository_ids: [repoB.id],
|
||||
});
|
||||
|
||||
// --- Land on kanban with workspace A active; task A visible, task B not ---
|
||||
const kanban = new KanbanPage(testPage);
|
||||
await kanban.goto();
|
||||
await expect(kanban.taskCard(taskA.id)).toBeVisible({ timeout: 10_000 });
|
||||
await expect(kanban.taskCard(taskB.id)).not.toBeVisible();
|
||||
|
||||
// --- Switch to workspace B via the display dropdown (SPA, no full reload) ---
|
||||
await testPage.getByTestId("display-button").click();
|
||||
await testPage.getByTestId("workspace-select-trigger").click();
|
||||
await testPage.getByTestId(`workspace-select-item-${workspaceB.id}`).click();
|
||||
|
||||
// Close the dropdown so task cards are interactable.
|
||||
await testPage.keyboard.press("Escape");
|
||||
|
||||
await expect(kanban.taskCard(taskB.id)).toBeVisible({ timeout: 10_000 });
|
||||
await expect(kanban.taskCard(taskA.id)).not.toBeVisible();
|
||||
|
||||
// --- Open task B; verify sidebar shows only workspace B's task ---
|
||||
await kanban.taskCard(taskB.id).click();
|
||||
const session = new SessionPage(testPage);
|
||||
await session.waitForLoad();
|
||||
await expect(session.sidebar).toBeVisible({ timeout: 10_000 });
|
||||
|
||||
await expect(session.sidebar.getByText("Workspace B Task", { exact: true })).toBeVisible({
|
||||
timeout: 10_000,
|
||||
});
|
||||
|
||||
await expect(session.sidebar.getByText("Workspace A Task", { exact: true })).toHaveCount(0);
|
||||
await expect(session.sidebar.getByTestId("sidebar-repo-group-Unassigned")).toHaveCount(0);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,145 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { renderHook, waitFor } from "@testing-library/react";
|
||||
|
||||
const mockClearKanbanMulti = vi.fn();
|
||||
const mockSetKanbanMultiLoading = vi.fn();
|
||||
const mockSetWorkflowSnapshot = vi.fn();
|
||||
const mockFetchWorkflowSnapshot = vi.fn();
|
||||
|
||||
type Workflow = { id: string; workspaceId: string; name: string };
|
||||
type MockState = {
|
||||
connection: { status: string };
|
||||
workflows: { items: Workflow[] };
|
||||
kanbanMulti: { snapshots: Record<string, unknown>; isLoading: boolean };
|
||||
clearKanbanMulti: typeof mockClearKanbanMulti;
|
||||
setKanbanMultiLoading: typeof mockSetKanbanMultiLoading;
|
||||
setWorkflowSnapshot: typeof mockSetWorkflowSnapshot;
|
||||
};
|
||||
|
||||
let mockState: MockState = {
|
||||
connection: { status: "connected" },
|
||||
workflows: { items: [] },
|
||||
kanbanMulti: { snapshots: {}, isLoading: false },
|
||||
clearKanbanMulti: mockClearKanbanMulti,
|
||||
setKanbanMultiLoading: mockSetKanbanMultiLoading,
|
||||
setWorkflowSnapshot: mockSetWorkflowSnapshot,
|
||||
};
|
||||
|
||||
vi.mock("@/components/state-provider", () => ({
|
||||
useAppStore: (selector: (s: MockState) => unknown) => selector(mockState),
|
||||
useAppStoreApi: () => ({ getState: () => mockState }),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/api", () => ({
|
||||
fetchWorkflowSnapshot: (...args: unknown[]) => mockFetchWorkflowSnapshot(...args),
|
||||
}));
|
||||
|
||||
import { useAllWorkflowSnapshots } from "./use-all-workflow-snapshots";
|
||||
|
||||
function resetMocks(workflows: Workflow[] = []) {
|
||||
vi.clearAllMocks();
|
||||
mockFetchWorkflowSnapshot.mockResolvedValue({ steps: [], tasks: [] });
|
||||
mockState = {
|
||||
connection: { status: "connected" },
|
||||
workflows: { items: workflows },
|
||||
kanbanMulti: { snapshots: {}, isLoading: false },
|
||||
clearKanbanMulti: mockClearKanbanMulti,
|
||||
setKanbanMultiLoading: mockSetKanbanMultiLoading,
|
||||
setWorkflowSnapshot: mockSetWorkflowSnapshot,
|
||||
};
|
||||
}
|
||||
|
||||
describe("useAllWorkflowSnapshots — workspace scoping", () => {
|
||||
beforeEach(() => {
|
||||
resetMocks([{ id: "wf-A", workspaceId: "ws-A", name: "A" }]);
|
||||
});
|
||||
|
||||
it("does not clear snapshots on initial mount (SSR preservation)", async () => {
|
||||
renderHook(
|
||||
({ workspaceId }: { workspaceId: string | null }) => useAllWorkflowSnapshots(workspaceId),
|
||||
{
|
||||
initialProps: { workspaceId: "ws-A" },
|
||||
},
|
||||
);
|
||||
|
||||
// Allow the effect + Promise.all to settle.
|
||||
await waitFor(() => expect(mockSetKanbanMultiLoading).toHaveBeenCalledWith(true));
|
||||
expect(mockClearKanbanMulti).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("clears snapshots when workspaceId changes", async () => {
|
||||
const { rerender } = renderHook(
|
||||
({ workspaceId }: { workspaceId: string | null }) => useAllWorkflowSnapshots(workspaceId),
|
||||
{ initialProps: { workspaceId: "ws-A" } },
|
||||
);
|
||||
await waitFor(() => expect(mockSetKanbanMultiLoading).toHaveBeenCalledWith(true));
|
||||
expect(mockClearKanbanMulti).not.toHaveBeenCalled();
|
||||
|
||||
// Switch to workspace B — must clear A's snapshots.
|
||||
mockState.workflows = { items: [{ id: "wf-B", workspaceId: "ws-B", name: "B" }] };
|
||||
rerender({ workspaceId: "ws-B" });
|
||||
|
||||
await waitFor(() => expect(mockClearKanbanMulti).toHaveBeenCalledTimes(1));
|
||||
});
|
||||
|
||||
it("skips refetch when workspace + workflow set is unchanged across renders", async () => {
|
||||
const workflows = [{ id: "wf-A", workspaceId: "ws-A", name: "A" }];
|
||||
const { rerender } = renderHook(
|
||||
({ workspaceId }: { workspaceId: string | null }) => useAllWorkflowSnapshots(workspaceId),
|
||||
{ initialProps: { workspaceId: "ws-A" } },
|
||||
);
|
||||
await waitFor(() => expect(mockFetchWorkflowSnapshot).toHaveBeenCalledTimes(1));
|
||||
|
||||
// Same-workflow rerender — dedup key unchanged, must not refetch.
|
||||
mockState.workflows = { items: [...workflows] };
|
||||
rerender({ workspaceId: "ws-A" });
|
||||
|
||||
// Positive signal: follow up with a DIFFERENT workflow set, which must
|
||||
// trigger a fetch. If dedup worked, the total is 2 (initial + this one).
|
||||
// If dedup failed, the same-key rerender would have fired a fetch before
|
||||
// this one, making the total 3. Waiting for count==2 proves both:
|
||||
// the dedup rerender was skipped AND the next real change still fetches.
|
||||
mockState.workflows = { items: [{ id: "wf-A2", workspaceId: "ws-A", name: "A2" }] };
|
||||
rerender({ workspaceId: "ws-A" });
|
||||
await waitFor(() => expect(mockFetchWorkflowSnapshot).toHaveBeenCalledTimes(2));
|
||||
expect(mockFetchWorkflowSnapshot.mock.calls[1][0]).toBe("wf-A2");
|
||||
expect(mockClearKanbanMulti).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("discards a stale in-flight fetch when workspace switches mid-fetch", async () => {
|
||||
// Hold the first fetch open so it resolves after the workspace switch.
|
||||
let resolveStale: (v: { steps: []; tasks: [] }) => void = () => {};
|
||||
mockFetchWorkflowSnapshot.mockImplementationOnce(
|
||||
() =>
|
||||
new Promise((res) => {
|
||||
resolveStale = res;
|
||||
}),
|
||||
);
|
||||
|
||||
const { rerender } = renderHook(
|
||||
({ workspaceId }: { workspaceId: string | null }) => useAllWorkflowSnapshots(workspaceId),
|
||||
{ initialProps: { workspaceId: "ws-A" } },
|
||||
);
|
||||
await waitFor(() =>
|
||||
expect(mockFetchWorkflowSnapshot).toHaveBeenCalledWith("wf-A", expect.anything()),
|
||||
);
|
||||
|
||||
// Switch to workspace B before A's fetch resolves. Wait for B's fetch
|
||||
// to settle (positive signal) so the new-gen effect is fully in place.
|
||||
mockFetchWorkflowSnapshot.mockResolvedValueOnce({ steps: [], tasks: [] });
|
||||
mockState.workflows = { items: [{ id: "wf-B", workspaceId: "ws-B", name: "B" }] };
|
||||
rerender({ workspaceId: "ws-B" });
|
||||
await waitFor(() =>
|
||||
expect(mockSetWorkflowSnapshot).toHaveBeenCalledWith("wf-B", expect.anything()),
|
||||
);
|
||||
|
||||
// Resolve A's stale fetch and drain the microtask queue so its .then
|
||||
// and .finally callbacks run. Flushing microtasks is deterministic —
|
||||
// unlike setTimeout, it doesn't depend on CI wall-clock speed.
|
||||
resolveStale({ steps: [], tasks: [] });
|
||||
for (let i = 0; i < 5; i++) await Promise.resolve();
|
||||
|
||||
const writtenIds = mockSetWorkflowSnapshot.mock.calls.map((args) => args[0]);
|
||||
expect(writtenIds).not.toContain("wf-A");
|
||||
});
|
||||
});
|
||||
@@ -1,10 +1,68 @@
|
||||
import { useEffect, useRef } from "react";
|
||||
import { useEffect, useRef, type MutableRefObject } from "react";
|
||||
import { fetchWorkflowSnapshot } from "@/lib/api";
|
||||
import { useAppStore, useAppStoreApi } from "@/components/state-provider";
|
||||
import type { KanbanState } from "@/lib/state/slices/kanban/types";
|
||||
import type { Task } from "@/lib/types/http";
|
||||
import type { StoreApi } from "zustand";
|
||||
import type { AppState } from "@/lib/state/store";
|
||||
|
||||
type KanbanTask = KanbanState["tasks"][number];
|
||||
type Workflow = { id: string; name: string };
|
||||
|
||||
async function fetchAndWriteSnapshot(
|
||||
wf: Workflow,
|
||||
store: StoreApi<AppState>,
|
||||
fetchGenRef: MutableRefObject<number>,
|
||||
myGen: number,
|
||||
): Promise<void> {
|
||||
try {
|
||||
const snapshot = await fetchWorkflowSnapshot(wf.id, { cache: "no-store" });
|
||||
if (fetchGenRef.current !== myGen) return;
|
||||
|
||||
const steps = snapshot.steps.map((step) => ({
|
||||
id: step.id,
|
||||
title: step.name,
|
||||
color: step.color ?? "bg-neutral-400",
|
||||
position: step.position,
|
||||
events: step.events,
|
||||
allow_manual_move: step.allow_manual_move,
|
||||
prompt: step.prompt,
|
||||
is_start_step: step.is_start_step,
|
||||
}));
|
||||
const stepIds = new Set(steps.map((s) => s.id));
|
||||
|
||||
// Preserve runtime fields (e.g., primarySessionId) from existing snapshot
|
||||
// tasks when the fresh API response omits them (backend uses omitempty).
|
||||
const existingSnapshot = store.getState().kanbanMulti.snapshots[wf.id];
|
||||
const existingById = new Map((existingSnapshot?.tasks ?? []).map((t) => [t.id, t]));
|
||||
|
||||
const tasks: KanbanTask[] = snapshot.tasks
|
||||
.filter((task) => !task.is_ephemeral)
|
||||
.map((task) => {
|
||||
const mapped = mapSnapshotTask(task, stepIds);
|
||||
if (!mapped) return null;
|
||||
const existing = existingById.get(mapped.id);
|
||||
if (existing) {
|
||||
mapped.primarySessionId = mapped.primarySessionId || existing.primarySessionId;
|
||||
mapped.primarySessionState = mapped.primarySessionState || existing.primarySessionState;
|
||||
}
|
||||
return mapped;
|
||||
})
|
||||
.filter((t): t is KanbanTask => t !== null);
|
||||
|
||||
store.getState().setWorkflowSnapshot(wf.id, {
|
||||
workflowId: wf.id,
|
||||
workflowName: wf.name,
|
||||
steps,
|
||||
tasks,
|
||||
});
|
||||
} catch (err) {
|
||||
console.error(
|
||||
`[useAllWorkflowSnapshots] Failed to fetch snapshot for workflow "${wf.name}" (${wf.id}):`,
|
||||
err,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// eslint-disable-next-line complexity -- pure field mapping, no real branching logic
|
||||
function mapSnapshotTask(task: Task, stepIds: Set<string>): KanbanTask | null {
|
||||
@@ -38,8 +96,20 @@ export function useAllWorkflowSnapshots(workspaceId: string | null) {
|
||||
const connectionStatus = useAppStore((state) => state.connection.status);
|
||||
const workflows = useAppStore((state) => state.workflows.items);
|
||||
const lastFetchedRef = useRef<string>("");
|
||||
const lastWorkspaceIdRef = useRef<string | null>(null);
|
||||
const fetchGenRef = useRef(0);
|
||||
|
||||
useEffect(() => {
|
||||
// Skip clear on initial mount to preserve SSR-hydrated snapshots.
|
||||
if (lastWorkspaceIdRef.current !== workspaceId) {
|
||||
if (lastWorkspaceIdRef.current !== null) {
|
||||
store.getState().clearKanbanMulti();
|
||||
lastFetchedRef.current = "";
|
||||
fetchGenRef.current += 1;
|
||||
}
|
||||
lastWorkspaceIdRef.current = workspaceId;
|
||||
}
|
||||
|
||||
if (!workspaceId) {
|
||||
return;
|
||||
}
|
||||
@@ -62,63 +132,13 @@ export function useAllWorkflowSnapshots(workspaceId: string | null) {
|
||||
}
|
||||
lastFetchedRef.current = key;
|
||||
|
||||
const { setKanbanMultiLoading, setWorkflowSnapshot } = store.getState();
|
||||
setKanbanMultiLoading(true);
|
||||
const myGen = fetchGenRef.current;
|
||||
store.getState().setKanbanMultiLoading(true);
|
||||
|
||||
Promise.all(
|
||||
workspaceWorkflows.map(async (wf) => {
|
||||
try {
|
||||
const snapshot = await fetchWorkflowSnapshot(wf.id, { cache: "no-store" });
|
||||
|
||||
const steps = snapshot.steps.map((step) => ({
|
||||
id: step.id,
|
||||
title: step.name,
|
||||
color: step.color ?? "bg-neutral-400",
|
||||
position: step.position,
|
||||
events: step.events,
|
||||
allow_manual_move: step.allow_manual_move,
|
||||
prompt: step.prompt,
|
||||
is_start_step: step.is_start_step,
|
||||
}));
|
||||
const stepIds = new Set(steps.map((s) => s.id));
|
||||
|
||||
// Preserve runtime fields (e.g., primarySessionId) from existing
|
||||
// snapshot tasks when the fresh API response omits them. The backend
|
||||
// uses omitempty for session fields, so a task whose session was just
|
||||
// created may not have primary_session_id in the snapshot response
|
||||
// yet, even though a WS task.updated event already delivered it.
|
||||
const existingSnapshot = store.getState().kanbanMulti.snapshots[wf.id];
|
||||
const existingById = new Map((existingSnapshot?.tasks ?? []).map((t) => [t.id, t]));
|
||||
|
||||
const tasks: KanbanTask[] = snapshot.tasks
|
||||
.filter((task) => !task.is_ephemeral) // Filter out ephemeral tasks (e.g., quick chat)
|
||||
.map((task) => {
|
||||
const mapped = mapSnapshotTask(task, stepIds);
|
||||
if (!mapped) return null;
|
||||
const existing = existingById.get(mapped.id);
|
||||
if (existing) {
|
||||
mapped.primarySessionId = mapped.primarySessionId || existing.primarySessionId;
|
||||
mapped.primarySessionState =
|
||||
mapped.primarySessionState || existing.primarySessionState;
|
||||
}
|
||||
return mapped;
|
||||
})
|
||||
.filter((t): t is KanbanTask => t !== null);
|
||||
|
||||
setWorkflowSnapshot(wf.id, {
|
||||
workflowId: wf.id,
|
||||
workflowName: wf.name,
|
||||
steps,
|
||||
tasks,
|
||||
});
|
||||
} catch (err) {
|
||||
console.error(
|
||||
`[useAllWorkflowSnapshots] Failed to fetch snapshot for workflow "${wf.name}" (${wf.id}):`,
|
||||
err,
|
||||
);
|
||||
}
|
||||
}),
|
||||
workspaceWorkflows.map((wf) => fetchAndWriteSnapshot(wf, store, fetchGenRef, myGen)),
|
||||
).finally(() => {
|
||||
if (fetchGenRef.current !== myGen) return;
|
||||
store.getState().setKanbanMultiLoading(false);
|
||||
});
|
||||
}, [workspaceId, workflows, connectionStatus, store]);
|
||||
|
||||
@@ -67,6 +67,7 @@ export const createKanbanSlice: StateCreator<
|
||||
clearKanbanMulti: () =>
|
||||
set((draft) => {
|
||||
draft.kanbanMulti.snapshots = {};
|
||||
draft.kanbanMulti.isLoading = false;
|
||||
}),
|
||||
updateMultiTask: (workflowId, task) =>
|
||||
set((draft) => {
|
||||
|
||||
Reference in New Issue
Block a user