diff --git a/apps/web/components/kanban-display-dropdown.tsx b/apps/web/components/kanban-display-dropdown.tsx index 6e73efa7b8..5ff7bf797f 100644 --- a/apps/web/components/kanban-display-dropdown.tsx +++ b/apps/web/components/kanban-display-dropdown.tsx @@ -41,12 +41,16 @@ function WorkspaceSection({ value={activeWorkspaceId ?? ""} onValueChange={(value) => onWorkspaceChange(value || null)} > - + {workspaces.map((workspace: Workspace) => ( - + {workspace.name} ))} diff --git a/apps/web/e2e/tests/task/workspace-switch-sidebar-isolation.spec.ts b/apps/web/e2e/tests/task/workspace-switch-sidebar-isolation.spec.ts new file mode 100644 index 0000000000..a77b5b389b --- /dev/null +++ b/apps/web/e2e/tests/task/workspace-switch-sidebar-isolation.spec.ts @@ -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); + }); +}); diff --git a/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.test.ts b/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.test.ts new file mode 100644 index 0000000000..268261aae7 --- /dev/null +++ b/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.test.ts @@ -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; 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"); + }); +}); diff --git a/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.ts b/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.ts index 1df19ac578..9672503865 100644 --- a/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.ts +++ b/apps/web/hooks/domains/kanban/use-all-workflow-snapshots.ts @@ -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, + fetchGenRef: MutableRefObject, + myGen: number, +): Promise { + 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): 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(""); + const lastWorkspaceIdRef = useRef(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]); diff --git a/apps/web/lib/state/slices/kanban/kanban-slice.ts b/apps/web/lib/state/slices/kanban/kanban-slice.ts index 3410b99d5b..c1e176b733 100644 --- a/apps/web/lib/state/slices/kanban/kanban-slice.ts +++ b/apps/web/lib/state/slices/kanban/kanban-slice.ts @@ -67,6 +67,7 @@ export const createKanbanSlice: StateCreator< clearKanbanMulti: () => set((draft) => { draft.kanbanMulti.snapshots = {}; + draft.kanbanMulti.isLoading = false; }), updateMultiTask: (workflowId, task) => set((draft) => {