diff --git a/apps/web/hooks/use-plan-panel-auto-open.test.ts b/apps/web/hooks/use-plan-panel-auto-open.test.ts index 67e24c2477..6b5d5af67b 100644 --- a/apps/web/hooks/use-plan-panel-auto-open.test.ts +++ b/apps/web/hooks/use-plan-panel-auto-open.test.ts @@ -4,25 +4,38 @@ import type { TaskPlan } from "@/lib/types/http"; const mockAddPlanPanel = vi.fn(); const mockGetPanel = vi.fn(); +const mockSetTaskPlan = vi.fn(); +const mockSetTaskPlanLoading = vi.fn(); +const mockMarkTaskPlanSeen = vi.fn(); +const mockGetTaskPlan = vi.fn(); let mockActiveTaskId: string | null = "task-1"; let mockPlan: TaskPlan | null = null; let mockLastSeen: string | undefined = undefined; +let mockIsLoaded = true; +let mockConnectionStatus = "connected"; let mockIsRestoringLayout = false; let mockApi: { getPanel: typeof mockGetPanel } | null = { getPanel: mockGetPanel }; +function buildState() { + return { + tasks: { activeTaskId: mockActiveTaskId }, + taskPlans: { + byTaskId: mockActiveTaskId && mockPlan ? { [mockActiveTaskId]: mockPlan } : {}, + loadedByTaskId: mockActiveTaskId ? { [mockActiveTaskId]: mockIsLoaded } : {}, + lastSeenUpdatedAtByTaskId: + mockActiveTaskId && mockLastSeen !== undefined ? { [mockActiveTaskId]: mockLastSeen } : {}, + }, + connection: { status: mockConnectionStatus }, + setTaskPlan: mockSetTaskPlan, + setTaskPlanLoading: mockSetTaskPlanLoading, + markTaskPlanSeen: mockMarkTaskPlanSeen, + }; +} + vi.mock("@/components/state-provider", () => ({ - useAppStore: (selector: (state: Record) => unknown) => - selector({ - tasks: { activeTaskId: mockActiveTaskId }, - taskPlans: { - byTaskId: mockActiveTaskId && mockPlan ? { [mockActiveTaskId]: mockPlan } : {}, - lastSeenUpdatedAtByTaskId: - mockActiveTaskId && mockLastSeen !== undefined - ? { [mockActiveTaskId]: mockLastSeen } - : {}, - }, - }), + useAppStore: (selector: (state: Record) => unknown) => selector(buildState()), + useAppStoreApi: () => ({ getState: () => buildState() }), })); vi.mock("@/lib/state/dockview-store", () => ({ @@ -34,6 +47,10 @@ vi.mock("@/lib/state/dockview-store", () => ({ }), })); +vi.mock("@/lib/api/domains/plan-api", () => ({ + getTaskPlan: (...args: unknown[]) => mockGetTaskPlan(...args), +})); + import { usePlanPanelAutoOpen } from "./use-plan-panel-auto-open"; const TS = "2026-04-20T00:00:00Z"; @@ -57,9 +74,12 @@ describe("usePlanPanelAutoOpen", () => { mockActiveTaskId = "task-1"; mockPlan = agentPlan(); mockLastSeen = undefined; + mockIsLoaded = true; + mockConnectionStatus = "connected"; mockIsRestoringLayout = false; mockApi = { getPanel: mockGetPanel }; mockGetPanel.mockReturnValue(null); + mockGetTaskPlan.mockResolvedValue(null); }); it("opens plan panel for unseen agent plan", () => { @@ -110,3 +130,141 @@ describe("usePlanPanelAutoOpen", () => { expect(mockAddPlanPanel).not.toHaveBeenCalled(); }); }); + +describe("usePlanPanelAutoOpen — eager fetch", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockActiveTaskId = "task-1"; + mockPlan = agentPlan(); + mockLastSeen = undefined; + mockIsLoaded = true; + mockConnectionStatus = "connected"; + mockIsRestoringLayout = false; + mockApi = { getPanel: mockGetPanel }; + mockGetPanel.mockReturnValue(null); + mockGetTaskPlan.mockResolvedValue(null); + }); + + it("eagerly fetches the plan when not yet loaded", () => { + mockIsLoaded = false; + mockPlan = null; + renderHook(() => usePlanPanelAutoOpen()); + expect(mockGetTaskPlan).toHaveBeenCalledWith("task-1"); + expect(mockSetTaskPlanLoading).toHaveBeenCalledWith("task-1", true); + }); + + it("does not fetch when WS is disconnected", () => { + mockIsLoaded = false; + mockConnectionStatus = "connecting"; + renderHook(() => usePlanPanelAutoOpen()); + expect(mockGetTaskPlan).not.toHaveBeenCalled(); + }); + + it("acknowledges the plan on hydrate when the panel was restored", () => { + mockGetPanel.mockReturnValue({ id: "plan" }); + mockLastSeen = undefined; + renderHook(() => usePlanPanelAutoOpen()); + expect(mockMarkTaskPlanSeen).toHaveBeenCalledWith("task-1"); + expect(mockAddPlanPanel).not.toHaveBeenCalled(); + }); + + it("does not acknowledge a live update when lastSeen is already recorded", () => { + mockGetPanel.mockReturnValue({ id: "plan" }); + mockLastSeen = TS; + mockPlan = agentPlan(TS_LATER); + renderHook(() => usePlanPanelAutoOpen()); + expect(mockMarkTaskPlanSeen).not.toHaveBeenCalled(); + expect(mockAddPlanPanel).not.toHaveBeenCalled(); + }); + + it("does not retry the eager fetch after a failure", async () => { + mockIsLoaded = false; + mockPlan = null; + let rejectFn: (err: Error) => void = () => {}; + mockGetTaskPlan.mockImplementation( + () => + new Promise((_, reject) => { + rejectFn = reject; + }), + ); + const { rerender } = renderHook(() => usePlanPanelAutoOpen()); + expect(mockGetTaskPlan).toHaveBeenCalledTimes(1); + rejectFn(new Error("boom")); + await new Promise((r) => setTimeout(r, 0)); + rerender(); + rerender(); + expect(mockGetTaskPlan).toHaveBeenCalledTimes(1); + }); +}); + +describe("usePlanPanelAutoOpen — race guards", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockActiveTaskId = "task-1"; + mockPlan = agentPlan(); + mockLastSeen = undefined; + mockIsLoaded = true; + mockConnectionStatus = "connected"; + mockIsRestoringLayout = false; + mockApi = { getPanel: mockGetPanel }; + mockGetPanel.mockReturnValue(null); + mockGetTaskPlan.mockResolvedValue(null); + }); + + it("does not overwrite a newer WS-delivered plan with an older HTTP result", async () => { + mockIsLoaded = false; + mockPlan = null; + let resolveFn: (v: TaskPlan | null) => void = () => {}; + mockGetTaskPlan.mockImplementation( + () => + new Promise((resolve) => { + resolveFn = resolve; + }), + ); + renderHook(() => usePlanPanelAutoOpen()); + // WS delivers the latest plan while the HTTP fetch is in flight. + mockPlan = agentPlan(TS_LATER); + // HTTP resolves with an older snapshot of the same plan. + resolveFn(agentPlan(TS)); + await new Promise((r) => setTimeout(r, 0)); + expect(mockSetTaskPlan).not.toHaveBeenCalled(); + }); + + it("does not overwrite a WS-delivered plan when the fetch resolves with null", async () => { + mockIsLoaded = false; + mockPlan = null; + let resolveFn: (v: TaskPlan | null) => void = () => {}; + mockGetTaskPlan.mockImplementation( + () => + new Promise((resolve) => { + resolveFn = resolve; + }), + ); + renderHook(() => usePlanPanelAutoOpen()); + // Simulate a WS event populating the store while the HTTP fetch is in flight. + mockPlan = agentPlan(); + resolveFn(null); + await new Promise((r) => setTimeout(r, 0)); + expect(mockSetTaskPlan).not.toHaveBeenCalled(); + }); + + it("retries the eager fetch after WS reconnects following a failure", async () => { + mockIsLoaded = false; + mockPlan = null; + mockGetTaskPlan.mockRejectedValueOnce(new Error("boom")); + mockGetTaskPlan.mockResolvedValueOnce(null); + + const { rerender } = renderHook(() => usePlanPanelAutoOpen()); + expect(mockGetTaskPlan).toHaveBeenCalledTimes(1); + await new Promise((r) => setTimeout(r, 0)); + + // WS disconnect — clears the attempted set + mockConnectionStatus = "connecting"; + rerender(); + + // WS reconnect — fetches again + mockConnectionStatus = "connected"; + rerender(); + expect(mockGetTaskPlan).toHaveBeenCalledTimes(2); + }); +}); diff --git a/apps/web/hooks/use-plan-panel-auto-open.ts b/apps/web/hooks/use-plan-panel-auto-open.ts index 6cdc396359..d6220aca1f 100644 --- a/apps/web/hooks/use-plan-panel-auto-open.ts +++ b/apps/web/hooks/use-plan-panel-auto-open.ts @@ -1,8 +1,9 @@ "use client"; -import { useEffect } from "react"; -import { useAppStore } from "@/components/state-provider"; +import { useEffect, useRef } from "react"; +import { useAppStore, useAppStoreApi } from "@/components/state-provider"; import { useDockviewStore } from "@/lib/state/dockview-store"; +import { getTaskPlan } from "@/lib/api/domains/plan-api"; /** * Watches the active task's plan and opens the Plan panel quietly (without @@ -18,19 +19,81 @@ import { useDockviewStore } from "@/lib/state/dockview-store"; export function usePlanPanelAutoOpen() { const activeTaskId = useAppStore((s) => s.tasks.activeTaskId); const plan = useAppStore((s) => (activeTaskId ? s.taskPlans.byTaskId[activeTaskId] : null)); + const isLoaded = useAppStore((s) => + activeTaskId ? (s.taskPlans.loadedByTaskId[activeTaskId] ?? false) : false, + ); const lastSeen = useAppStore((s) => activeTaskId ? s.taskPlans.lastSeenUpdatedAtByTaskId[activeTaskId] : undefined, ); + const setTaskPlan = useAppStore((s) => s.setTaskPlan); + const setTaskPlanLoading = useAppStore((s) => s.setTaskPlanLoading); + const markTaskPlanSeen = useAppStore((s) => s.markTaskPlanSeen); + const connectionStatus = useAppStore((s) => s.connection.status); + const storeApi = useAppStoreApi(); const api = useDockviewStore((s) => s.api); const isRestoringLayout = useDockviewStore((s) => s.isRestoringLayout); const addPlanPanel = useDockviewStore((s) => s.addPlanPanel); + // Track tasks we've already attempted to fetch so a transient failure + // doesn't put us in an infinite retry loop. Cleared on WS disconnect so a + // reconnect can retry any failed or not-yet-attempted tasks. + const attemptedRef = useRef>(new Set()); + + // Eagerly fetch the plan on task load. The Plan panel mounts `useTaskPlan` + // only after the panel exists, so without this fetch a plan written by the + // agent before the browser's WS connected (fast auto-start path) would never + // populate the store and the auto-open below would never fire. + useEffect(() => { + if (!activeTaskId || connectionStatus !== "connected") return; + if (isLoaded) return; + if (attemptedRef.current.has(activeTaskId)) return; + const taskId = activeTaskId; + attemptedRef.current.add(taskId); + setTaskPlanLoading(taskId, true); + getTaskPlan(taskId) + .then((fetched) => { + // Race guard: if a WS event populated the store while our HTTP + // request was in flight, don't overwrite a real plan with a + // stale response — neither a `null` (server didn't have it yet + // at fetch time) nor an older non-null version (HTTP saw an + // earlier write than the WS event we already applied). + const live = storeApi.getState().taskPlans.byTaskId[taskId]; + if (live) { + if (fetched === null) return; + if (Date.parse(fetched.updated_at) < Date.parse(live.updated_at)) return; + } + setTaskPlan(taskId, fetched); + }) + .catch(() => { + /* swallow — the disconnect/reconnect effect clears `attemptedRef` + * so a transient failure retries automatically after recovery. */ + }) + .finally(() => setTaskPlanLoading(taskId, false)); + }, [activeTaskId, connectionStatus, isLoaded, setTaskPlan, setTaskPlanLoading, storeApi]); + + // Clear the attempt set on WS disconnect so that when the WS reconnects + // the fetch effect can retry any tasks that previously failed or were pending. + useEffect(() => { + if (connectionStatus === "connected") return; + attemptedRef.current.clear(); + }, [connectionStatus]); + useEffect(() => { if (!api || isRestoringLayout) return; if (!plan || plan.created_by !== "agent") return; if (lastSeen === plan.updated_at) return; - if (api.getPanel("plan")) return; + if (api.getPanel("plan")) { + // Page-reload case: panel restored from saved layout and there is no + // recorded `lastSeen` (not persisted across sessions). We can't tell + // whether the plan was acknowledged before reload or not, so we + // optimistically mark it seen to avoid a stale indicator flash on + // every reload. When a live update arrives after the reload, + // `lastSeen` is defined so we don't suppress it — PlanTab reads the + // store directly and re-arms the indicator. + if (lastSeen === undefined) markTaskPlanSeen(plan.task_id); + return; + } addPlanPanel({ quiet: true, inCenter: true }); - }, [api, isRestoringLayout, plan, lastSeen, addPlanPanel]); + }, [api, isRestoringLayout, plan, lastSeen, addPlanPanel, markTaskPlanSeen]); }