diff --git a/plugins/tasks/shell/app-shell.tsx b/plugins/tasks/shell/app-shell.tsx index 365038ea08..98b911eae6 100644 --- a/plugins/tasks/shell/app-shell.tsx +++ b/plugins/tasks/shell/app-shell.tsx @@ -298,7 +298,14 @@ function TasksAppShellContent({ subPath }: PluginNavPanelProps) { summaries={summaries.data} presets={presets.data} activeTasks={activeTasks.data} - isLoading={projects.isLoading || summaries.isLoading} + // Skeleton only while there is nothing to draw: a refetch (manual + // refresh, invalidation) keeps the last-known rows on screen, and a + // snapshot-hydrated mount never shows the placeholder at all. + isLoading={ + folders.data === undefined || + projects.data === undefined || + summaries.data === undefined + } overlay={sidebarOverlay} onNavigate={navigateFromSidebar} onNewProject={() => setNewProjectOpen(true)} diff --git a/plugins/tasks/shell/data.ts b/plugins/tasks/shell/data.ts index 9d2e4df087..0d24474f4c 100644 --- a/plugins/tasks/shell/data.ts +++ b/plugins/tasks/shell/data.ts @@ -1,9 +1,11 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { useRealtime, useRpc } from "@get-bb/plugin-sdk/app"; -import type { TasksRpcContract } from "../shared/contract.js"; +import type { z } from "zod"; +import { tasksRpcContract, type TasksRpcContract } from "../shared/contract.js"; import type { Task, TaskPriority, TaskStatus } from "../shared/contract.js"; import { TASKS_PAGE_MAX_LIMIT, type TaskSort } from "../shared/pagination.js"; import type { MentionItem } from "../editor/extensions.js"; +import { readQuerySnapshot, writeQuerySnapshot } from "./query-snapshot.js"; import { useTasksRefresh } from "./refresh.js"; /** Typed RPC client bound to the tasks contract. */ @@ -88,21 +90,46 @@ export interface TasksQuery { * refresh provider so the header control can single-flight without a timer. * Invalidation- and deps-driven refetches do not mark the shared in-flight bit. */ +export interface TasksQuerySnapshot { + /** Storage name; the value is validated against `schema` on every read. */ + name: string; + schema: z.ZodType; +} + export function useTasksQuery( fetcher: (rpc: TasksRpc) => Promise, channels: readonly InvalidationChannel[], deps: readonly unknown[] = [], + options: { + /** + * Seed the first render with the last result this browser saw for the + * query and keep that record fresh after every successful fetch. Use for + * small, shape-stable results whose absence forces a wrong-shaped loading + * UI (the sidebar's project list, its counts). `isLoading` still reports + * the in-flight fetch; only `data` starts populated. + */ + snapshot?: TasksQuerySnapshot; + } = {}, ): TasksQuery { const rpc = useTasksRpc(); const { generation, beginGenerationWork, endGenerationWork } = useTasksRefresh(); const fetcherRef = useRef(fetcher); fetcherRef.current = fetcher; + const snapshotRef = useRef(options.snapshot); + snapshotRef.current = options.snapshot; const [state, setState] = useState<{ data: T | undefined; error: string | null; isLoading: boolean; - }>({ data: undefined, error: null, isLoading: true }); + }>(() => ({ + data: + options.snapshot === undefined + ? undefined + : readQuerySnapshot(options.snapshot.name, options.snapshot.schema), + error: null, + isLoading: true, + })); const seqRef = useRef(0); const previousGenerationRef = useRef(generation); const depsKey = JSON.stringify(deps); @@ -111,6 +138,8 @@ export function useTasksQuery( return fetcherRef.current(rpc).then( (data) => { if (seq !== seqRef.current) return; + const snapshot = snapshotRef.current; + if (snapshot !== undefined) writeQuerySnapshot(snapshot.name, data); setState({ data, error: null, isLoading: false }); }, (error: unknown) => { @@ -143,17 +172,31 @@ export function useTasksQuery( return { ...state, refresh }; } +const foldersSnapshot = { + name: "folders", + schema: tasksRpcContract.listFolders.output.shape.folders, +}; + export function useFolders() { return useTasksQuery( async (rpc) => (await rpc.call("listFolders")).folders, ["projects:changed"], + [], + { snapshot: foldersSnapshot }, ); } +const projectsSnapshot = { + name: "projects", + schema: tasksRpcContract.listProjects.output.shape.projects, +}; + export function useProjects() { return useTasksQuery( async (rpc) => (await rpc.call("listProjects", {})).projects, ["projects:changed"], + [], + { snapshot: projectsSnapshot }, ); } @@ -164,10 +207,17 @@ export function usePresets() { ); } +const sidebarSummarySnapshot = { + name: "sidebar-summary", + schema: tasksRpcContract.sidebarSummary.output.shape.projects, +}; + export function useSidebarSummary() { return useTasksQuery( async (rpc) => (await rpc.call("sidebarSummary")).projects, ["tasks:changed", "projects:changed", "threads:changed"], + [], + { snapshot: sidebarSummarySnapshot }, ); } diff --git a/plugins/tasks/shell/query-snapshot.ts b/plugins/tasks/shell/query-snapshot.ts new file mode 100644 index 0000000000..ea642fecd3 --- /dev/null +++ b/plugins/tasks/shell/query-snapshot.ts @@ -0,0 +1,76 @@ +import type { z } from "zod"; + +const QUERY_SNAPSHOT_STORAGE_ROOT = "bb-tasks:query-snapshot:"; +const QUERY_SNAPSHOT_STORAGE_VERSION = "v1"; +const QUERY_SNAPSHOT_STORAGE_PREFIX = `${QUERY_SNAPSHOT_STORAGE_ROOT}${QUERY_SNAPSHOT_STORAGE_VERSION}:`; + +export function querySnapshotStorageKey(name: string): string { + return `${QUERY_SNAPSHOT_STORAGE_PREFIX}${name}`; +} + +let prunedOtherVersions = false; + +/** Test-only: forget that this page load already pruned. */ +export function resetQuerySnapshotStateForTest(): void { + prunedOtherVersions = false; +} + +/** + * A version bump changes the key prefix, so older entries are simply never + * read again — but they would sit in the profile forever. Drop them once per + * page load, on the first snapshot access. + */ +function pruneOtherSnapshotVersions(): void { + if (prunedOtherVersions) return; + prunedOtherVersions = true; + try { + const stale: string[] = []; + for (let index = 0; index < window.localStorage.length; index += 1) { + const key = window.localStorage.key(index); + if ( + key !== null && + key.startsWith(QUERY_SNAPSHOT_STORAGE_ROOT) && + !key.startsWith(QUERY_SNAPSHOT_STORAGE_PREFIX) + ) { + stale.push(key); + } + } + for (const key of stale) window.localStorage.removeItem(key); + } catch { + // No storage, or none we may enumerate: nothing to prune. + } +} + +/** + * Last-known query results, kept in the browser profile so a remount paints + * the same truth it showed last time instead of a loading placeholder that + * the real rows then replace. localStorage is a system boundary: anything + * that fails to parse against the query's own schema is treated as absent. + * Storage failures (disabled, full, private mode) degrade to "no snapshot". + */ +export function readQuerySnapshot( + name: string, + schema: z.ZodType, +): T | undefined { + pruneOtherSnapshotVersions(); + try { + const raw = window.localStorage.getItem(querySnapshotStorageKey(name)); + if (raw === null) return undefined; + const parsed = schema.safeParse(JSON.parse(raw)); + return parsed.success ? parsed.data : undefined; + } catch { + return undefined; + } +} + +export function writeQuerySnapshot(name: string, value: unknown): void { + pruneOtherSnapshotVersions(); + try { + window.localStorage.setItem( + querySnapshotStorageKey(name), + JSON.stringify(value), + ); + } catch { + // Best-effort: the next mount simply loads without a snapshot. + } +} diff --git a/plugins/tasks/shell/shell.test.tsx b/plugins/tasks/shell/shell.test.tsx index 4c33cf3de3..0236e50b95 100644 --- a/plugins/tasks/shell/shell.test.tsx +++ b/plugins/tasks/shell/shell.test.tsx @@ -1,5 +1,5 @@ // @vitest-environment jsdom -import { cleanup, fireEvent, waitFor } from "@testing-library/react"; +import { act, cleanup, fireEvent, waitFor } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { loadPluginApp, renderSlot } from "@get-bb/plugin-sdk/testing/app"; @@ -24,6 +24,8 @@ const { parseTasksRoute, tasksRouteToSubPath } = await import("./routes.js"); const { pagerPosition } = await import("./topbar.js"); const { SIDEBAR_COLLAPSED_STORAGE_KEY } = await import("./sidebar-preference.js"); +const { querySnapshotStorageKey, resetQuerySnapshotStateForTest } = + await import("./query-snapshot.js"); beforeEach(() => window.localStorage.clear()); afterEach(() => { @@ -658,6 +660,133 @@ describe("tasks app shell", () => { ); }); + describe("last-known snapshot", () => { + const projectsKey = querySnapshotStorageKey("projects"); + const foldersKey = querySnapshotStorageKey("folders"); + const summaryKey = querySnapshotStorageKey("sidebar-summary"); + const summary = { + projectId: PROJECT_ID, + taskCount: 3, + activeAgentCount: 1, + }; + // An RPC the test settles by hand, so the pre-resolution render is observable. + function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((r) => { + resolve = r; + }); + return { promise, resolve }; + } + + it("never paints the empty state while projects are unknown", async () => { + const projects = deferred<{ projects: never[] }>(); + const slot = renderSlot( + app.navPanels[0]!, + { subPath: "" }, + { + rpc: seededRpc({ + listProjects: () => projects.promise, + sidebarSummary: () => ({ projects: [] }), + }), + }, + ); + // Cold profile: emptiness is not known yet, so the empty state must wait. + expect(slot.queryByText("No projects yet")).toBeNull(); + projects.resolve({ projects: [] }); + await slot.findByText("No projects yet"); + }); + + it("paints the last-known empty state before listProjects resolves", () => { + window.localStorage.setItem(projectsKey, JSON.stringify([])); + window.localStorage.setItem(foldersKey, JSON.stringify([])); + window.localStorage.setItem(summaryKey, JSON.stringify([])); + const projects = deferred<{ projects: never[] }>(); + const slot = renderSlot( + app.navPanels[0]!, + { subPath: "" }, + { + rpc: seededRpc({ + listProjects: () => projects.promise, + sidebarSummary: () => ({ projects: [] }), + }), + }, + ); + // First paint already matches the last truth this browser saw: no list chrome first. + expect(slot.getByText("No projects yet")).toBeTruthy(); + }); + + it("paints last-known projects before listProjects resolves and never flashes empty", async () => { + window.localStorage.setItem(projectsKey, JSON.stringify([project])); + window.localStorage.setItem(foldersKey, JSON.stringify([folder])); + window.localStorage.setItem(summaryKey, JSON.stringify([summary])); + const projects = deferred<{ projects: (typeof project)[] }>(); + const slot = renderSlot( + app.navPanels[0]!, + { subPath: "" }, + { rpc: seededRpc({ listProjects: () => projects.promise }) }, + ); + expect(slot.getByText(project.name)).toBeTruthy(); + expect(slot.queryByText("No projects yet")).toBeNull(); + projects.resolve({ projects: [project] }); + await waitFor(() => expect(slot.getByText(project.name)).toBeTruthy()); + expect(slot.queryByText("No projects yet")).toBeNull(); + }); + + it("ignores a malformed snapshot and loads normally", async () => { + window.localStorage.setItem(projectsKey, "{not json"); + window.localStorage.setItem( + summaryKey, + JSON.stringify([{ projectId: 1 }]), + ); + const slot = renderSlot( + app.navPanels[0]!, + { subPath: "" }, + { rpc: emptyRpc }, + ); + expect(slot.queryByText("No projects yet")).toBeNull(); + await slot.findByText("No projects yet"); + }); + + it("prunes snapshots written under an older storage version", async () => { + // Pruning runs once per page load; this test owns a fresh load. + resetQuerySnapshotStateForTest(); + window.localStorage.setItem( + "bb-tasks:query-snapshot:v0:projects", + JSON.stringify([]), + ); + const slot = renderSlot( + app.navPanels[0]!, + { subPath: "" }, + { rpc: seededRpc() }, + ); + await slot.findByText(project.name); + expect( + window.localStorage.getItem("bb-tasks:query-snapshot:v0:projects"), + ).toBeNull(); + expect(window.localStorage.getItem(projectsKey)).not.toBeNull(); + }); + + it("records the fetched projects and counts for the next mount", async () => { + const slot = renderSlot( + app.navPanels[0]!, + { subPath: "" }, + { rpc: seededRpc() }, + ); + await slot.findByText(project.name); + await waitFor(() => { + expect( + JSON.parse(window.localStorage.getItem(projectsKey) ?? "null"), + ).toEqual([project]); + expect( + JSON.parse(window.localStorage.getItem(foldersKey) ?? "null"), + ).toEqual([folder]); + expect( + JSON.parse(window.localStorage.getItem(summaryKey) ?? "null"), + ).toEqual([summary]); + }); + }); + }); + it("shows the empty state and opens the New project dialog", async () => { const slot = renderSlot( app.navPanels[0]!, @@ -671,6 +800,50 @@ describe("tasks app shell", () => { await slot.findByText("Projects group tasks under a shared key prefix."); }); + it("does not paint another scope's empty state while its own rows load", async () => { + // All tasks has rows; Active has none. Switching Active back to All keeps + // the same ListView instance, whose query still holds Active's empty + // result while All refetches; the body must read as loading, never as + // "No tasks yet", until All's own rows settle. + const tasks = [ + { + ...pagerTask("TSK-4", "todo", 1), + title: "Scope truth", + description: "", + labelIds: [], + }, + ]; + let deferAll = false; + let releaseAll: (() => void) | null = null; + const rpc = seededRpc({ + listLabels: () => ({ labels: [] }), + // The shell's own Active count also calls listTasks (activeOnly), so + // route by arguments rather than call order. + listTasks: (input: { activeOnly?: boolean }) => { + if (input.activeOnly === true) return { tasks: [] }; + if (!deferAll) return { tasks }; + return new Promise((resolve) => { + releaseAll = () => resolve({ tasks }); + }); + }, + }); + const Panel = app.navPanels[0]!.component; + const slot = renderSlot(app.navPanels[0]!, { subPath: "all" }, { rpc }); + await slot.findByText("Scope truth"); + + slot.lifecycle.rerender(); + await slot.findByText("No agents working right now"); + + deferAll = true; + slot.lifecycle.rerender(); + await waitFor(() => expect(releaseAll).not.toBeNull()); + // In flight: Active's emptiness must not masquerade as All's. + expect(slot.queryByText("No tasks yet")).toBeNull(); + expect(slot.queryByText("Scope truth")).toBeNull(); + act(() => releaseAll!()); + await slot.findByText("Scope truth"); + }); + it("renders sidebar data and routes project/board/task subPaths", async () => { const boardSlot = renderSlot( app.navPanels[0]!, diff --git a/plugins/tasks/views/list/index.tsx b/plugins/tasks/views/list/index.tsx index 29de212b08..fd6ce7f7d0 100644 --- a/plugins/tasks/views/list/index.tsx +++ b/plugins/tasks/views/list/index.tsx @@ -215,6 +215,28 @@ export function ListView({ projectId, activeOnly = false }: ListViewProps) { useEffect(() => { if (!tasksQuery.isLoading) settledScope.current = scopeKey; }, [scopeKey, tasksQuery.isLoading, tasksQuery.data]); + // The route scope is the fetch identity across views: All, Active, or one + // project. Switching it reuses this ListView instance, whose query still + // holds the previous route's result, so the body below must read as loading + // until this route's own fetch settles: returning from an empty Active to + // All must not present Active's emptiness as "No tasks yet". Narrower than + // `scopeKey` on purpose, so filter and sort changes keep painting the rows + // they already have. State, not a ref: settling has to rerender the body. + const routeScope = `${projectId ?? "-"}/${activeOnly}`; + const [settledRouteScope, setSettledRouteScope] = useState(routeScope); + const routeScopeChanged = settledRouteScope !== routeScope; + const previousRouteScope = useRef(routeScope); + useEffect(() => { + // The query's own effect flips `isLoading` in this same commit, but this + // effect still reads the previous render's value, so the commit that + // changes the route scope must never settle it; a later resolved commit + // does. + const routeScopeJustChanged = previousRouteScope.current !== routeScope; + previousRouteScope.current = routeScope; + if (!routeScopeJustChanged && !tasksQuery.isLoading) { + setSettledRouteScope(routeScope); + } + }, [routeScope, tasksQuery.isLoading, tasksQuery.data]); useListScrollRestoration(scrollRef, scopeKey, { contentReady: tasksQuery.data !== undefined && tasksQuery.data.length > 0, loading: tasksQuery.isLoading || scopeChanged, @@ -222,9 +244,16 @@ export function ListView({ projectId, activeOnly = false }: ListViewProps) { }); let body: React.ReactNode; - if (tasksQuery.data === undefined || displayTasks === undefined) { + if ( + routeScopeChanged || + tasksQuery.data === undefined || + displayTasks === undefined + ) { + // While a changed route scope is in flight, any held data or error is the + // previous route's; only a settled result may claim this scope is empty + // or broken. body = - tasksQuery.error !== null ? ( + !routeScopeChanged && tasksQuery.error !== null ? ( { + if (typeof window !== "undefined") window.localStorage.clear(); +});