diff --git a/apps/web/components/task/mobile/session-task-switcher-sheet-hooks.ts b/apps/web/components/task/mobile/session-task-switcher-sheet-hooks.ts index 04611835b1..325c23b7de 100644 --- a/apps/web/components/task/mobile/session-task-switcher-sheet-hooks.ts +++ b/apps/web/components/task/mobile/session-task-switcher-sheet-hooks.ts @@ -87,11 +87,10 @@ export function useSheetData(workspaceId: string | null, workflowId: string | nu const gitStatusByEnvId = useAppStore((state) => state.gitStatus.byEnvironmentId); const envIdBySessionId = useAppStore((state) => state.environmentIdBySessionId); const messagesBySession = useAppStore((state) => state.messages.bySession); - const { tasks } = useTasks(workflowId); + const { tasks, isLoading: tasksLoading } = useTasks(workflowId); const steps = useAppStore((state) => state.kanban.steps); const workspaces = useAppStore((state) => state.workspaces.items); const repositoriesByWorkspace = useAppStore((state) => state.repositories.itemsByWorkspaceId); - const kanbanIsLoading = useAppStore((state) => state.kanban.isLoading ?? false); const selectedTaskId = useMemo(() => { if (activeSessionId) return sessionsById[activeSessionId]?.task_id ?? activeTaskId; @@ -157,7 +156,8 @@ export function useSheetData(workspaceId: string | null, workflowId: string | nu selectedTaskId, steps, workspaces, - kanbanIsLoading, + // Skeleton while snapshot hydrates kanban — otherwise shows "No tasks yet." even when tasks exist. + tasksLoading, tasksWithRepositories, dialogSteps, }; diff --git a/apps/web/components/task/mobile/session-task-switcher-sheet.tsx b/apps/web/components/task/mobile/session-task-switcher-sheet.tsx index 509ced78ce..08e11ecad4 100644 --- a/apps/web/components/task/mobile/session-task-switcher-sheet.tsx +++ b/apps/web/components/task/mobile/session-task-switcher-sheet.tsx @@ -114,7 +114,7 @@ export const SessionTaskSwitcherSheet = memo(function SessionTaskSwitcherSheet({ onArchiveTask={actions.handleArchiveTask} onDeleteTask={actions.handleDeleteTask} deletingTaskId={actions.deletingTaskId} - isLoading={data.kanbanIsLoading} + isLoading={data.tasksLoading} /> diff --git a/apps/web/hooks/use-tasks.test.ts b/apps/web/hooks/use-tasks.test.ts new file mode 100644 index 0000000000..24987e05fe --- /dev/null +++ b/apps/web/hooks/use-tasks.test.ts @@ -0,0 +1,121 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { renderHook } from "@testing-library/react"; + +const mockHydrate = vi.fn(); +const mockSetState = vi.fn(); +const mockFetchWorkflowSnapshot = vi.fn(); + +type Task = { id: string; title: string }; +type MockState = { + connection: { status: string }; + kanban: { workflowId: string | null; tasks: Task[]; steps: unknown[]; isLoading?: boolean }; + hydrate: typeof mockHydrate; +}; + +let mockState: MockState = { + connection: { status: "connected" }, + kanban: { workflowId: null, tasks: [], steps: [] }, + hydrate: mockHydrate, +}; + +vi.mock("@/components/state-provider", () => ({ + useAppStore: (selector: (s: MockState) => unknown) => selector(mockState), + useAppStoreApi: () => ({ + getState: () => mockState, + setState: (updater: (s: MockState) => MockState) => { + // Apply the updater so useWorkflowSnapshot's isLoading transitions are + // visible to subsequent reads in the same render cycle. Without this + // the mock would silently swallow side effects. + mockState = updater(mockState); + mockSetState(mockState); + }, + }), +})); + +vi.mock("@/lib/api", () => ({ + fetchWorkflowSnapshot: (...args: unknown[]) => mockFetchWorkflowSnapshot(...args), +})); + +vi.mock("@/lib/ssr/mapper", () => ({ + snapshotToState: (snapshot: unknown) => ({ snapshot }), +})); + +import { useTasks } from "./use-tasks"; + +function setMockState(patch: Partial & { workflowId?: string | null }) { + mockState = { + ...mockState, + kanban: { ...mockState.kanban, ...patch }, + }; +} + +describe("useTasks — loading and matching", () => { + beforeEach(() => { + vi.clearAllMocks(); + // Never resolves so we observe pre-resolution loading state. + mockFetchWorkflowSnapshot.mockReturnValue(new Promise(() => {})); + mockState = { + connection: { status: "connected" }, + kanban: { workflowId: null, tasks: [], steps: [], isLoading: false }, + hydrate: mockHydrate, + }; + }); + + it("returns empty list and not-loading when workflowId is null", () => { + const { result } = renderHook(() => useTasks(null)); + expect(result.current.tasks).toEqual([]); + expect(result.current.isLoading).toBe(false); + }); + + it("reports loading once useWorkflowSnapshot has flipped kanban.isLoading", () => { + setMockState({ workflowId: null, isLoading: true }); + const { result } = renderHook(() => useTasks("wf-1")); + expect(result.current.tasks).toEqual([]); + expect(result.current.isLoading).toBe(true); + }); + + it("does not report loading when fetch has settled but workflowId mismatches", () => { + // Simulates a failed snapshot fetch — `kanban.isLoading` is back to false + // and `kanban.workflowId` never advanced to the requested id. Caller + // shows the empty-state UI rather than an infinite skeleton. + setMockState({ workflowId: null, isLoading: false }); + const { result } = renderHook(() => useTasks("wf-1")); + expect(result.current.tasks).toEqual([]); + expect(result.current.isLoading).toBe(false); + }); + + it("returns tasks and not-loading once kanban.workflowId matches", () => { + setMockState({ + workflowId: "wf-1", + tasks: [ + { id: "t1", title: "One" }, + { id: "t2", title: "Two" }, + ], + isLoading: false, + }); + const { result } = renderHook(() => useTasks("wf-1")); + expect(result.current.tasks).toHaveLength(2); + expect(result.current.isLoading).toBe(false); + }); + + it("filters out tasks that belong to a different workflow snapshot", () => { + setMockState({ + workflowId: "wf-other", + tasks: [{ id: "t1", title: "One" }], + isLoading: false, + }); + const { result } = renderHook(() => useTasks("wf-1")); + expect(result.current.tasks).toEqual([]); + expect(result.current.isLoading).toBe(false); + }); + + it("respects kanban.isLoading even when workflowId already matches", () => { + setMockState({ + workflowId: "wf-1", + tasks: [{ id: "t1", title: "One" }], + isLoading: true, + }); + const { result } = renderHook(() => useTasks("wf-1")); + expect(result.current.isLoading).toBe(true); + }); +}); diff --git a/apps/web/hooks/use-tasks.ts b/apps/web/hooks/use-tasks.ts index a5e2e898f5..6823ed26f7 100644 --- a/apps/web/hooks/use-tasks.ts +++ b/apps/web/hooks/use-tasks.ts @@ -6,14 +6,14 @@ export function useTasks(workflowId: string | null) { useWorkflowSnapshot(workflowId); const kanbanWorkflowId = useAppStore((state) => state.kanban.workflowId); + const kanbanIsLoading = useAppStore((state) => state.kanban.isLoading ?? false); const tasks = useAppStore((state) => state.kanban.tasks); - const workflowTasks = useMemo(() => { - if (!workflowId || kanbanWorkflowId !== workflowId) { - return []; - } - return tasks; - }, [workflowId, kanbanWorkflowId, tasks]); + const matchesActive = !!workflowId && kanbanWorkflowId === workflowId; + const workflowTasks = useMemo(() => (matchesActive ? tasks : []), [matchesActive, tasks]); - return { tasks: workflowTasks }; + // Loading only while a snapshot fetch is in-flight; settles to false on success/error to avoid an infinite skeleton. + const isLoading = !!workflowId && kanbanIsLoading; + + return { tasks: workflowTasks, isLoading }; } diff --git a/apps/web/hooks/use-workflow-snapshot.test.ts b/apps/web/hooks/use-workflow-snapshot.test.ts new file mode 100644 index 0000000000..014b6bc4d1 --- /dev/null +++ b/apps/web/hooks/use-workflow-snapshot.test.ts @@ -0,0 +1,139 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { renderHook, waitFor } from "@testing-library/react"; + +const mockHydrate = vi.fn(); +const mockFetchWorkflowSnapshot = vi.fn(); +const mockSetState = vi.fn(); + +type MockKanban = { + workflowId: string | null; + tasks: unknown[]; + steps: unknown[]; + isLoading?: boolean; +}; +type MockState = { + connection: { status: string }; + kanban: MockKanban; + hydrate: typeof mockHydrate; +}; + +let mockState: MockState = { + connection: { status: "connected" }, + kanban: { workflowId: null, tasks: [], steps: [], isLoading: false }, + hydrate: mockHydrate, +}; + +vi.mock("@/components/state-provider", () => ({ + useAppStore: (selector: (s: MockState) => unknown) => selector(mockState), + useAppStoreApi: () => ({ + getState: () => mockState, + setState: (updater: (s: MockState) => MockState) => { + const next = updater(mockState); + mockSetState(next); + mockState = next; + }, + }), +})); + +vi.mock("@/lib/api", () => ({ + fetchWorkflowSnapshot: (...args: unknown[]) => mockFetchWorkflowSnapshot(...args), +})); + +vi.mock("@/lib/ssr/mapper", () => ({ + snapshotToState: (snapshot: unknown) => ({ snapshot }), +})); + +import { useWorkflowSnapshot } from "./use-workflow-snapshot"; + +function resetState(kanban: Partial = {}) { + vi.clearAllMocks(); + mockState = { + connection: { status: "connected" }, + kanban: { + workflowId: null, + tasks: [], + steps: [], + isLoading: false, + ...kanban, + }, + hydrate: mockHydrate, + }; +} + +describe("useWorkflowSnapshot — kanban.isLoading", () => { + beforeEach(() => resetState()); + + it("flips isLoading true while a fetch for an un-hydrated workflow is in flight", () => { + mockFetchWorkflowSnapshot.mockReturnValue(new Promise(() => {})); + renderHook(() => useWorkflowSnapshot("wf-1")); + expect(mockState.kanban.isLoading).toBe(true); + }); + + it("flips isLoading back to false after the fetch resolves", async () => { + mockFetchWorkflowSnapshot.mockResolvedValue({ steps: [], tasks: [] }); + renderHook(() => useWorkflowSnapshot("wf-1")); + await waitFor(() => expect(mockHydrate).toHaveBeenCalled()); + await waitFor(() => expect(mockState.kanban.isLoading).toBe(false)); + }); + + it("flips isLoading back to false after the fetch rejects", async () => { + mockFetchWorkflowSnapshot.mockRejectedValue(new Error("nope")); + renderHook(() => useWorkflowSnapshot("wf-1")); + await waitFor(() => expect(mockState.kanban.isLoading).toBe(false)); + expect(mockHydrate).not.toHaveBeenCalled(); + }); + + it("does not flip isLoading true if the requested workflow is already hydrated", () => { + resetState({ workflowId: "wf-1", isLoading: false }); + mockFetchWorkflowSnapshot.mockReturnValue(new Promise(() => {})); + renderHook(() => useWorkflowSnapshot("wf-1")); + expect(mockState.kanban.isLoading).toBe(false); + }); + + it("does not clear isLoading on settle when it didn't raise the flag (already-hydrated re-fetch)", async () => { + // Mimic a workspace switch having set isLoading=true after the snapshot + // hydrated for this workflowId. A silent re-fetch (e.g. WS reconnect) + // must not collapse that skeleton. + resetState({ workflowId: "wf-1", isLoading: true }); + mockFetchWorkflowSnapshot.mockResolvedValue({ steps: [], tasks: [] }); + renderHook(() => useWorkflowSnapshot("wf-1")); + await waitFor(() => expect(mockHydrate).toHaveBeenCalled()); + expect(mockState.kanban.isLoading).toBe(true); + }); + + it("does nothing when workflowId is null", () => { + renderHook(() => useWorkflowSnapshot(null)); + expect(mockFetchWorkflowSnapshot).not.toHaveBeenCalled(); + expect(mockState.kanban.isLoading).toBe(false); + }); + + it("does not clear isLoading when an old fetch settles after workflowId changes", async () => { + // First fetch never settles synchronously — we resolve it manually + // *after* re-rendering with a new workflowId, simulating the race where + // the user switches workflows mid-fetch. + let resolveFirst!: (snapshot: { steps: unknown[]; tasks: unknown[] }) => void; + const firstFetch = new Promise<{ steps: unknown[]; tasks: unknown[] }>((r) => { + resolveFirst = r; + }); + const secondFetch = new Promise<{ steps: unknown[]; tasks: unknown[] }>(() => {}); + mockFetchWorkflowSnapshot.mockReturnValueOnce(firstFetch).mockReturnValueOnce(secondFetch); + + const { rerender } = renderHook(({ id }: { id: string | null }) => useWorkflowSnapshot(id), { + initialProps: { id: "wf-1" as string | null }, + }); + expect(mockState.kanban.isLoading).toBe(true); + + // User switches to wf-2 before wf-1 finishes loading + rerender({ id: "wf-2" }); + expect(mockState.kanban.isLoading).toBe(true); + + // Old fetch lands now; flush its .then/.finally microtask chain so we + // can assert the negatives without relying on waitFor (which would + // resolve immediately for never-true assertions). + resolveFirst({ steps: [], tasks: [] }); + await Promise.resolve(); + await Promise.resolve(); + expect(mockHydrate).not.toHaveBeenCalled(); + expect(mockState.kanban.isLoading).toBe(true); + }); +}); diff --git a/apps/web/hooks/use-workflow-snapshot.ts b/apps/web/hooks/use-workflow-snapshot.ts index 511ac55d8e..d18fb9935d 100644 --- a/apps/web/hooks/use-workflow-snapshot.ts +++ b/apps/web/hooks/use-workflow-snapshot.ts @@ -9,12 +9,28 @@ export function useWorkflowSnapshot(workflowId: string | null) { useEffect(() => { if (!workflowId) return; + let cancelled = false; + const setLoading = store.getState().kanban.workflowId !== workflowId; + if (setLoading) { + store.setState((state) => ({ ...state, kanban: { ...state.kanban, isLoading: true } })); + } fetchWorkflowSnapshot(workflowId, { cache: "no-store" }) .then((snapshot) => { + if (cancelled) return; store.getState().hydrate(snapshotToState(snapshot)); }) - .catch(() => { - // Ignore snapshot errors — will retry on WS reconnect. + .catch((error) => { + // Suppress superseded-fetch noise; retry happens on WS reconnect. + if (cancelled) return; + console.warn("[useWorkflowSnapshot] failed to load snapshot:", error); + }) + .finally(() => { + // Only clear the flag this effect raised; skip when cancelled or when a concurrent caller owns it. + if (cancelled || !setLoading) return; + store.setState((state) => ({ ...state, kanban: { ...state.kanban, isLoading: false } })); }); + return () => { + cancelled = true; + }; }, [workflowId, store, connectionStatus]); }