From dc74e8871e8aa3d9bc5d729faa6cbc14331abb40 Mon Sep 17 00:00:00 2001 From: niufuti-cyber <274451909+niufuti-cyber@users.noreply.github.com> Date: Mon, 29 Jun 2026 07:40:16 +0800 Subject: [PATCH] refactor(agents): adopt useApi on detail page --- README.md | 6 + src/app/agents/[agent]/page.test.tsx | 248 +++++++++++++++++++++++++++ src/app/agents/[agent]/page.tsx | 33 ++-- src/app/changelog/page.test.tsx | 20 ++- src/lib/useApi.ts | 4 +- 5 files changed, 297 insertions(+), 14 deletions(-) create mode 100644 src/app/agents/[agent]/page.test.tsx diff --git a/README.md b/README.md index f6ea571..a429061 100644 --- a/README.md +++ b/README.md @@ -129,6 +129,12 @@ See [docs/hooks.md](docs/hooks.md) for the shared hook reference, including signatures, return shapes, cancellation and SSR notes, and usage examples for the hooks in `src/lib`. +The agent detail route uses `useApi` for its primary usage request, keyed by the +URL-encoded agent identifier. Navigating between agents aborts the superseded +usage request and ignores any stale completion. Its optional lifetime-total +request remains a soft failure, but is guarded so a slower previous agent cannot +overwrite the current agent's total. + ## Error boundaries ### Route-level boundary (`src/app/error.tsx`) diff --git a/src/app/agents/[agent]/page.test.tsx b/src/app/agents/[agent]/page.test.tsx new file mode 100644 index 0000000..508e036 --- /dev/null +++ b/src/app/agents/[agent]/page.test.tsx @@ -0,0 +1,248 @@ +import { render, screen, waitFor } from "@testing-library/react"; +import AgentDetailPage from "./page"; +import { apiGet } from "@/lib/apiClient"; + +jest.mock("@/lib/apiClient", () => ({ + apiGet: jest.fn(), +})); + +jest.mock("react", () => { + const originalReact = jest.requireActual("react"); + return { + ...originalReact, + use: (usable: unknown) => { + const u = usable as { _value?: unknown } | null | undefined; + if (u && u._value) { + return u._value; + } + return originalReact.use(usable); + }, + }; +}); + +const apiGetMock = apiGet as jest.MockedFunction; + +type Usage = { + agent: string; + items: { serviceId: string; total: number }[]; +}; + +type Deferred = { + promise: Promise; + resolve: (value: T) => void; + reject: (error: Error) => void; +}; + +function deferred(): Deferred { + let resolve!: (value: T) => void; + let reject!: (error: Error) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +function paramsFor(agent: string) { + const params = Promise.resolve({ agent }) as Promise<{ agent: string }> & { + _value: { agent: string }; + }; + params._value = { agent }; + return params; +} + +function renderPage(agent: string) { + return render(); +} + +function signalFor(path: string) { + const call = apiGetMock.mock.calls.find(([calledPath]) => calledPath === path); + const init = call?.[1] as RequestInit | undefined; + return init?.signal ?? undefined; +} + +describe("AgentDetailPage", () => { + beforeEach(() => { + apiGetMock.mockReset(); + }); + + it("renders usage rows and the optional lifetime total when both requests succeed", async () => { + apiGetMock.mockImplementation((path: string) => { + if (path === "/api/v1/agents/agent-alpha/usage") { + return Promise.resolve({ + agent: "agent-alpha", + items: [{ serviceId: "svc-1", total: 12 }], + } satisfies Usage) as never; + } + if (path === "/api/v1/agents/agent-alpha/total") { + return Promise.resolve({ total: 42 }) as never; + } + return Promise.reject(new Error(`unexpected path: ${path}`)) as never; + }); + + renderPage("agent-alpha"); + + expect( + await screen.findByRole("heading", { name: "agent-alpha" }), + ).toBeInTheDocument(); + expect(await screen.findByText("svc-1")).toBeInTheDocument(); + expect(screen.getByText("12 requests")).toBeInTheDocument(); + expect(screen.getByText(/Lifetime total:/)).toHaveTextContent( + "Lifetime total: 42 requests", + ); + }); + + it("surfaces usage failures as a role=alert", async () => { + apiGetMock.mockImplementation((path: string) => { + if (path.endsWith("/usage")) { + return Promise.reject(new Error("Backend usage offline")) as never; + } + if (path.endsWith("/total")) { + return Promise.resolve({ total: 10 }) as never; + } + return Promise.reject(new Error(`unexpected path: ${path}`)) as never; + }); + + renderPage("agent-error"); + + expect(await screen.findByRole("alert")).toHaveTextContent( + "Backend usage offline", + ); + }); + + it("keeps the optional total request as a soft failure", async () => { + apiGetMock.mockImplementation((path: string) => { + if (path.endsWith("/usage")) { + return Promise.resolve({ + agent: "agent-soft-total", + items: [], + } satisfies Usage) as never; + } + if (path.endsWith("/total")) { + return Promise.reject(new Error("total unavailable")) as never; + } + return Promise.reject(new Error(`unexpected path: ${path}`)) as never; + }); + + renderPage("agent-soft-total"); + + expect( + await screen.findByText("No services consumed yet."), + ).toBeInTheDocument(); + expect(screen.queryByRole("alert")).not.toBeInTheDocument(); + expect(screen.queryByText(/Lifetime total:/)).not.toBeInTheDocument(); + }); + + it("ignores stale usage and total responses after a rapid agent switch", async () => { + const alphaUsage = deferred(); + const alphaTotal = deferred<{ total: number }>(); + const betaUsage = deferred(); + const betaTotal = deferred<{ total: number }>(); + + apiGetMock.mockImplementation((path: string) => { + if (path === "/api/v1/agents/agent-alpha/usage") { + return alphaUsage.promise as never; + } + if (path === "/api/v1/agents/agent-alpha/total") { + return alphaTotal.promise as never; + } + if (path === "/api/v1/agents/agent-beta/usage") { + return betaUsage.promise as never; + } + if (path === "/api/v1/agents/agent-beta/total") { + return betaTotal.promise as never; + } + return Promise.reject(new Error(`unexpected path: ${path}`)) as never; + }); + + const { rerender } = renderPage("agent-alpha"); + + await waitFor(() => { + expect(signalFor("/api/v1/agents/agent-alpha/usage")).toEqual( + expect.any(AbortSignal), + ); + expect( + apiGetMock.mock.calls.some( + ([path]) => path === "/api/v1/agents/agent-alpha/total", + ), + ).toBe(true); + }); + const alphaUsageSignal = signalFor("/api/v1/agents/agent-alpha/usage"); + expect(alphaUsageSignal?.aborted).toBe(false); + + rerender(); + + await waitFor(() => { + expect(signalFor("/api/v1/agents/agent-beta/usage")).toEqual( + expect.any(AbortSignal), + ); + }); + expect(alphaUsageSignal?.aborted).toBe(true); + + betaUsage.resolve({ + agent: "agent-beta", + items: [{ serviceId: "svc-latest", total: 7 }], + }); + betaTotal.resolve({ total: 70 }); + + expect(await screen.findByText("svc-latest")).toBeInTheDocument(); + expect(screen.getByText("7 requests")).toBeInTheDocument(); + expect(screen.getByText(/Lifetime total:/)).toHaveTextContent( + "Lifetime total: 70 requests", + ); + + alphaUsage.resolve({ + agent: "agent-alpha", + items: [{ serviceId: "svc-stale", total: 99 }], + }); + alphaTotal.resolve({ total: 990 }); + + await waitFor(() => { + expect(screen.getByText("svc-latest")).toBeInTheDocument(); + expect(screen.queryByText("svc-stale")).not.toBeInTheDocument(); + expect(screen.getByText(/Lifetime total:/)).toHaveTextContent( + "Lifetime total: 70 requests", + ); + }); + }); + + it("aborts the useApi usage request and avoids state updates after unmount", async () => { + const usage = deferred(); + const total = deferred<{ total: number }>(); + let usageSignal: AbortSignal | undefined; + const consoleErrorSpy = jest.spyOn(console, "error").mockImplementation(() => { + /* silence React error logging */ + }); + + apiGetMock.mockImplementation((path: string, init?: RequestInit) => { + if (path.endsWith("/usage")) { + usageSignal = init?.signal ?? undefined; + return usage.promise as never; + } + if (path.endsWith("/total")) { + return total.promise as never; + } + return Promise.reject(new Error(`unexpected path: ${path}`)) as never; + }); + + const { unmount } = renderPage("agent-unmount"); + + await waitFor(() => { + expect(usageSignal).toBeDefined(); + }); + + unmount(); + expect(usageSignal?.aborted).toBe(true); + + try { + usage.resolve({ agent: "agent-unmount", items: [] }); + total.resolve({ total: 5 }); + + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(consoleErrorSpy).not.toHaveBeenCalled(); + } finally { + consoleErrorSpy.mockRestore(); + } + }); +}); diff --git a/src/app/agents/[agent]/page.tsx b/src/app/agents/[agent]/page.tsx index dc3cf4d..98e98f6 100644 --- a/src/app/agents/[agent]/page.tsx +++ b/src/app/agents/[agent]/page.tsx @@ -1,10 +1,12 @@ "use client"; -import { useEffect, useState, use } from "react"; +import { use, useEffect, useState } from "react"; import Link from "next/link"; import { apiGet } from "@/lib/apiClient"; +import { useApi } from "@/lib/useApi"; type Usage = { agent: string; items: { serviceId: string; total: number }[] }; +type TotalState = { agent: string; total: number } | null; export default function AgentDetailPage({ params, @@ -12,22 +14,29 @@ export default function AgentDetailPage({ params: Promise<{ agent: string }>; }) { const { agent } = use(params); - const [items, setItems] = useState(null); - const [total, setTotal] = useState(null); - const [error, setError] = useState(null); + const encodedAgent = encodeURIComponent(agent); + const usageState = useApi(`/api/v1/agents/${encodedAgent}/usage`); + const [totalState, setTotalState] = useState(null); useEffect(() => { - apiGet(`/api/v1/agents/${encodeURIComponent(agent)}/usage`) - .then((b) => setItems(b.items)) - .catch((e) => setError(e.message)); - apiGet<{ total: number }>( - `/api/v1/agents/${encodeURIComponent(agent)}/total` - ) - .then((b) => setTotal(b.total)) + let cancelled = false; + + apiGet<{ total: number }>(`/api/v1/agents/${encodedAgent}/total`) + .then((b) => { + if (!cancelled) setTotalState({ agent, total: b.total }); + }) .catch(() => { /* total is optional */ }); - }, [agent]); + + return () => { + cancelled = true; + }; + }, [agent, encodedAgent]); + + const items = usageState.status === "ok" ? usageState.data.items : null; + const error = usageState.status === "error" ? usageState.error : null; + const total = totalState?.agent === agent ? totalState.total : null; return (
{ render(); - expect(mockApiGet).toHaveBeenCalledWith("/api/v1/changelog"); + expect(mockApiGet).toHaveBeenCalledWith( + "/api/v1/changelog", + expect.objectContaining({ signal: expect.any(AbortSignal) }), + ); expect(screen.getByRole("status")).toHaveTextContent("Loading changelog"); }); + it("aborts the shared changelog request on unmount", () => { + mockApiGet.mockReturnValue(new Promise(() => {})); + + const { unmount } = render(); + const init = mockApiGet.mock.calls[0]?.[1] as RequestInit | undefined; + const signal = init?.signal; + + expect(signal).toEqual(expect.any(AbortSignal)); + expect(signal?.aborted).toBe(false); + + unmount(); + + expect(signal?.aborted).toBe(true); + }); + it("renders changelog entries on success", async () => { mockApiGet.mockResolvedValue({ entries: [ diff --git a/src/lib/useApi.ts b/src/lib/useApi.ts index 6a5a96f..b5ebd74 100644 --- a/src/lib/useApi.ts +++ b/src/lib/useApi.ts @@ -29,9 +29,10 @@ export function useApi(path: string | null): State { useEffect(() => { if (path === null) return; + const controller = new AbortController(); let cancelled = false; dispatch({ status: "loading" }); - apiGet(path) + apiGet(path, { signal: controller.signal }) .then((data) => !cancelled && dispatch({ status: "ok", data })) .catch( (e) => @@ -43,6 +44,7 @@ export function useApi(path: string | null): State { ); return () => { cancelled = true; + controller.abort(); }; }, [path]);