From 3272affbb389f4d54a2ab7794337008392e61e55 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 16 Aug 2026 01:50:38 -0700 Subject: [PATCH] FN-9120: Fence Create Room agent roster loads Keep Create Room picker state accurate across overlapping agent roster requests. - Track explicit idle, loading, loaded, and failed picker phases. - Ignore stale close, reopen, project-change, and unmount request completions. - Reconcile selected members and cover ordering, failure, empty, duplicate, desktop, and mobile states. - Document the loaded-lane flake investigation and add a patch changeset. Files changed: .changeset/fn-9120-create-room-picker.md | 7 ++ .../suite-only-flakes-observed-register.md | 17 +++ .../dashboard/app/components/CreateRoomModal.tsx | 50 ++++++-- .../components/__tests__/CreateRoomModal.test.tsx | 126 ++++++++++++++++----- 4 files changed, 165 insertions(+), 35 deletions(-) Fusion-Task-Id: FN-9120 Fusion-Task-Lineage: 5c9ff011-7653-40ab-a7c1-e3464ca3eaf5 Co-authored-by: Fusion (runfusion.ai) --- .changeset/fn-9120-create-room-picker.md | 7 + .../suite-only-flakes-observed-register.md | 17 +++ .../app/components/CreateRoomModal.tsx | 52 ++++++-- .../__tests__/CreateRoomModal.test.tsx | 124 ++++++++++++++---- 4 files changed, 165 insertions(+), 35 deletions(-) create mode 100644 .changeset/fn-9120-create-room-picker.md diff --git a/.changeset/fn-9120-create-room-picker.md b/.changeset/fn-9120-create-room-picker.md new file mode 100644 index 0000000000..281394edaa --- /dev/null +++ b/.changeset/fn-9120-create-room-picker.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Keep Create Room member picker states accurate while agent data loads. +category: fix +dev: Fence superseded agent roster requests and distinguish loading, empty, and failed picker states. diff --git a/docs/solutions/test-failures/suite-only-flakes-observed-register.md b/docs/solutions/test-failures/suite-only-flakes-observed-register.md index b607c39563..d5cbadf0c8 100644 --- a/docs/solutions/test-failures/suite-only-flakes-observed-register.md +++ b/docs/solutions/test-failures/suite-only-flakes-observed-register.md @@ -170,3 +170,20 @@ FN-9116 adds deterministic ordering coverage for desktop and mobile rows across | `pnpm lint`, `pnpm verify:fast`, `pnpm build` | **passed** | The flake is structurally removed rather than stabilized: every hydration/recovery writer now has an ownership boundary before it can overwrite a newer turn. This is a published behavior fix, so FN-9116 includes a patch changeset. + +## 9. Create Room picker loaded-lane state ordering + +- **File:** `packages/dashboard/app/components/__tests__/CreateRoomModal.test.tsx` +- **Exact test:** `CreateRoomModal > shows loading, empty, no-match, populated, and selected-member picker states` +- **Observed tree/SHA:** `7527d2651f` (FN-9120 baseline). +- **Observed frequency:** 2/2 loaded `dashboard-app-quality-backfill` shard-2 runs failed; a targeted rerun had passed before this investigation. + +| run | result | +|---|---| +| loaded backfill shard 2 | **failed** — 1 failed / 2,134 passed; full output retained | +| loaded backfill shard 2 with picker instrumentation | **failed** — same assertion; fetch calls were exactly 1/2/3 and the member list still rendered Alpha/Beta after typing `zzz` | +| new ordering tests against unfixed component | **failed** — stale project result overwrote current roster; rejected load rendered no-agents copy | + +**Resolved 2026-08-16 (FN-9120): both a timing-sensitive test assertion and a product race.** The original third phase synchronously asserted after `userEvent.type` while the loaded lane still rendered populated rows, even though its once queue had not shifted. Independently, the production effect had no cleanup or request identity, so a close/reopen, project change, or unmount could let a stale fetch write roster/loading/error state; initial `loadingAgents=false` also exposed terminal empty copy before the first effect. + +The component now owns an explicit idle/loading/loaded/failed phase and fences each request with an epoch plus cleanup. A current successful reload removes selected IDs absent from its roster. The test uses controlled deferred promises in a single persistently-mounted modal, proves close/reopen/project ordering, failure and unmount fencing, duplicate-name/selection reconciliation, and desktop/mobile empty-state copy invariants without retries, sleeps, waits around the old assertion, or mock re-pinning. diff --git a/packages/dashboard/app/components/CreateRoomModal.tsx b/packages/dashboard/app/components/CreateRoomModal.tsx index 707328b9a4..db8e8b95ea 100644 --- a/packages/dashboard/app/components/CreateRoomModal.tsx +++ b/packages/dashboard/app/components/CreateRoomModal.tsx @@ -45,11 +45,12 @@ export function CreateRoomModal({ isOpen, onClose, onCreate, projectId, existing const [agents, setAgents] = useState([]); const [search, setSearch] = useState(""); const [selectedAgentIds, setSelectedAgentIds] = useState([]); - const [loadingAgents, setLoadingAgents] = useState(false); + const [agentLoadPhase, setAgentLoadPhase] = useState<"idle" | "loading" | "loaded" | "failed">("idle"); const [submitError, setSubmitError] = useState(null); const [isSubmitting, setIsSubmitting] = useState(false); const nameInputRef = useRef(null); const previousFocusRef = useRef(null); + const agentLoadEpochRef = useRef(0); /* FNXC:ModalTouchGeometry 2026-07-26-19:25: Create Room is a blocking child of Quick Chat. The shared utility layer now claims its fresh @@ -57,17 +58,39 @@ export function CreateRoomModal({ isOpen, onClose, onCreate, projectId, existing */ useEffect(() => { + const requestEpoch = ++agentLoadEpochRef.current; if (!isOpen) return; + previousFocusRef.current = document.activeElement instanceof HTMLElement ? document.activeElement : null; - setLoadingAgents(true); + setAgents([]); + setAgentLoadPhase("loading"); setSubmitError(null); - fetchAgents(undefined, projectId) - .then((result) => setAgents(result)) - .catch(() => { - setAgents([]); - setSubmitError(t("createRoom.failedLoadAgents", "Failed to load agents.")); + let cancelled = false; + const isCurrentRequest = () => !cancelled && agentLoadEpochRef.current === requestEpoch; + + /* + FNXC:CreateRoomModal 2026-08-16-08:11: + The modal stays mounted while Chat toggles it and can switch projects mid-request. Only the + latest open request may publish its roster, error, or phase; cleanup fences close, unmount, + and project-change completions before they can overwrite the current picker. + */ + void fetchAgents(undefined, projectId) + .then((result) => { + if (!isCurrentRequest()) return; + setAgents(result); + setSelectedAgentIds((selected) => selected.filter((id) => result.some((agent) => agent.id === id))); + setAgentLoadPhase("loaded"); }) - .finally(() => setLoadingAgents(false)); + .catch(() => { + if (!isCurrentRequest()) return; + setAgents([]); + setAgentLoadPhase("failed"); + }); + + return () => { + cancelled = true; + ++agentLoadEpochRef.current; + }; }, [isOpen, projectId]); useEffect(() => { @@ -75,6 +98,8 @@ export function CreateRoomModal({ isOpen, onClose, onCreate, projectId, existing setRawName(""); setSearch(""); setSelectedAgentIds([]); + setAgents([]); + setAgentLoadPhase("idle"); setSubmitError(null); setIsSubmitting(false); return; @@ -115,7 +140,7 @@ export function CreateRoomModal({ isOpen, onClose, onCreate, projectId, existing [agents, selectedAgentIds], ); - const canSubmit = validation.ok && selectedAgentIds.length > 0 && !isSubmitting && !loadingAgents; + const canSubmit = validation.ok && selectedAgentIds.length > 0 && !isSubmitting && agentLoadPhase === "loaded"; if (!isOpen) return null; @@ -232,8 +257,15 @@ export function CreateRoomModal({ isOpen, onClose, onCreate, projectId, existing lists preserve their independent scroll behavior inside the movable dialog. */}
- {loadingAgents ? ( + {/* + FNXC:CreateRoomModal 2026-08-16-08:11: + Empty copy is meaningful only after the current request has settled. Until then show the + loading status, and distinguish a failed request from an actually empty project roster. + */} + {agentLoadPhase === "idle" || agentLoadPhase === "loading" ? (
+ ) : agentLoadPhase === "failed" ? ( +
{t("createRoom.failedLoadAgents", "Failed to load agents.")}
) : filteredAgents.length === 0 ? (
{agents.length === 0 ? t("createRoom.noAgents", "No agents in this project yet.") : t("createRoom.noMatch", "No agents match your search.")} diff --git a/packages/dashboard/app/components/__tests__/CreateRoomModal.test.tsx b/packages/dashboard/app/components/__tests__/CreateRoomModal.test.tsx index 294fcc6d95..bc7aa7d88e 100644 --- a/packages/dashboard/app/components/__tests__/CreateRoomModal.test.tsx +++ b/packages/dashboard/app/components/__tests__/CreateRoomModal.test.tsx @@ -1,4 +1,4 @@ -import { fireEvent, render, screen, waitFor, within } from "@testing-library/react"; +import { act, fireEvent, render, screen, waitFor, within } from "@testing-library/react"; import { describe, expect, it, vi, beforeEach } from "vitest"; import { userEvent } from "@testing-library/user-event"; import { CreateRoomModal, validateRoomName } from "../CreateRoomModal"; @@ -11,6 +11,20 @@ vi.mock("../../api", () => ({ })); const mockFetchAgents = vi.mocked(apiModule.fetchAgents); +const agents = [ + { id: "agent-1", name: "Alpha", role: "executor", state: "idle", metadata: {}, createdAt: "", updatedAt: "" }, + { id: "agent-2", name: "Beta", role: "reviewer", state: "idle", metadata: {}, createdAt: "", updatedAt: "" }, +] as any; + +function createDeferred() { + let resolve!: (value: T) => void; + let reject!: (reason?: unknown) => void; + const promise = new Promise((resolvePromise, rejectPromise) => { + resolve = resolvePromise; + reject = rejectPromise; + }); + return { promise, resolve, reject }; +} describe("validateRoomName", () => { it.each([ @@ -40,10 +54,7 @@ describe("CreateRoomModal", () => { beforeEach(() => { vi.clearAllMocks(); localStorage.clear(); - mockFetchAgents.mockResolvedValue([ - { id: "agent-1", name: "Alpha", role: "executor", state: "idle", metadata: {}, createdAt: "", updatedAt: "" }, - { id: "agent-2", name: "Beta", role: "reviewer", state: "idle", metadata: {}, createdAt: "", updatedAt: "" }, - ] as any); + mockFetchAgents.mockResolvedValue(agents); }); it("renders nothing when closed", () => { @@ -186,36 +197,99 @@ describe("CreateRoomModal", () => { await waitFor(() => expect(screen.getByRole("button", { name: "Room launcher" })).toHaveFocus()); }); - it("shows loading, empty, no-match, populated, and selected-member picker states", async () => { - mockFetchAgents.mockImplementationOnce(() => new Promise(() => {})); - const loading = render(); - expect(await screen.findByRole("status")).toHaveTextContent("Loading agents..."); - loading.unmount(); + it.each(["desktop", "mobile"])("shows loading, empty, no-match, populated, and selected-member picker states on %s", async (viewport) => { + Object.defineProperty(window, "innerWidth", { configurable: true, value: viewport === "mobile" ? 375 : 1280 }); + const loading = createDeferred(); + const empty = createDeferred(); + const populated = createDeferred(); + mockFetchAgents + .mockImplementationOnce(() => loading.promise) + .mockImplementationOnce(() => empty.promise) + .mockImplementationOnce(() => populated.promise); + const { rerender } = render(); - mockFetchAgents.mockResolvedValueOnce([]); - const empty = render(); - expect(await screen.findByText("No agents in this project yet.")).toBeInTheDocument(); - empty.unmount(); + expect(screen.getByRole("status")).toHaveTextContent("Loading agents..."); + expect(mockFetchAgents).toHaveBeenCalledTimes(1); + await act(async () => { loading.resolve(agents); }); + expect(await screen.findByRole("button", { name: /Alpha/i })).toBeInTheDocument(); - // Re-pin the populated list after mockResolvedValueOnce([]) so the no-match path cannot race an empty load. - mockFetchAgents.mockResolvedValue([ - { id: "agent-1", name: "Alpha", role: "executor", state: "idle", metadata: {}, createdAt: "", updatedAt: "" }, - { id: "agent-2", name: "Beta", role: "reviewer", state: "idle", metadata: {}, createdAt: "", updatedAt: "" }, - ] as any); - render(); - await screen.findByRole("button", { name: /Alpha/i }); - await userEvent.type(screen.getByLabelText("Members"), "zzz"); + rerender(); + rerender(); + await waitFor(() => expect(mockFetchAgents).toHaveBeenCalledTimes(2)); + await act(async () => { empty.resolve([]); }); + expect(screen.getByText("No agents in this project yet.")).toBeInTheDocument(); + expect(screen.queryByText("No agents match your search.")).not.toBeInTheDocument(); + + rerender(); + rerender(); + await waitFor(() => expect(mockFetchAgents).toHaveBeenCalledTimes(3)); + await act(async () => { populated.resolve(agents); }); + expect(await screen.findByRole("button", { name: /Alpha/i })).toBeInTheDocument(); + fireEvent.change(screen.getByLabelText("Members"), { target: { value: "zzz" } }); expect(screen.getByText("No agents match your search.")).toBeInTheDocument(); - await userEvent.clear(screen.getByLabelText("Members")); - await userEvent.click(await screen.findByRole("button", { name: /Alpha/i })); + expect(screen.queryByText("No agents in this project yet.")).not.toBeInTheDocument(); + fireEvent.change(screen.getByLabelText("Members"), { target: { value: "" } }); + await userEvent.click(screen.getByRole("button", { name: /Alpha/i })); expect(screen.getByTestId("create-room-selected-chips")).toHaveTextContent("Alpha"); + expect(mockFetchAgents).toHaveBeenCalledTimes(3); + }); + + it("ignores a superseded close/reopen and project-change load on the persistently mounted modal", async () => { + const projectA = createDeferred(); + const projectB = createDeferred(); + mockFetchAgents.mockImplementationOnce(() => projectA.promise).mockImplementationOnce(() => projectB.promise); + const { rerender } = render(); + rerender(); + rerender(); + await waitFor(() => expect(mockFetchAgents).toHaveBeenCalledTimes(2)); + + await act(async () => { projectB.resolve([agents[1]]); }); + expect(await screen.findByRole("button", { name: /Beta/i })).toBeInTheDocument(); + await act(async () => { projectA.resolve(agents); }); + expect(screen.queryByRole("button", { name: /Alpha/i })).not.toBeInTheDocument(); + expect(screen.getByRole("button", { name: /Beta/i })).toBeInTheDocument(); + expect(mockFetchAgents.mock.calls.map(([, projectId]) => projectId)).toEqual(["project-a", "project-b"]); + }); + + it("drops selected ids missing from a current reload while retaining duplicate-name rows", async () => { + const initial = createDeferred(); + const reloaded = createDeferred(); + const duplicateNameAgents = [agents[0], { ...agents[0], id: "agent-3" }]; + mockFetchAgents.mockImplementationOnce(() => initial.promise).mockImplementationOnce(() => reloaded.promise); + const { rerender } = render(); + await act(async () => { initial.resolve(duplicateNameAgents); }); + expect(await screen.findAllByRole("button", { name: /Alpha/i })).toHaveLength(2); + await userEvent.click(screen.getAllByRole("button", { name: /Alpha/i })[0]); + expect(screen.getByTestId("create-room-selected-chips")).toHaveTextContent("Alpha"); + + rerender(); + await act(async () => { reloaded.resolve([agents[1]]); }); + expect(screen.queryByTestId("create-room-selected-chips")).not.toBeInTheDocument(); + expect(screen.getByRole("button", { name: /Beta/i })).toBeInTheDocument(); + }); + + it("keeps a rejected load distinct from an empty roster and fences an unmounted load", async () => { + const rejected = createDeferred(); + mockFetchAgents.mockImplementationOnce(() => rejected.promise); + const { unmount } = render(); + await act(async () => { rejected.reject(new Error("offline")); }); + expect(await screen.findByText("Failed to load agents.")).toBeInTheDocument(); + expect(screen.queryByText("No agents in this project yet.")).not.toBeInTheDocument(); + + const unmounted = createDeferred(); + mockFetchAgents.mockImplementationOnce(() => unmounted.promise); + unmount(); + const second = render(); + second.unmount(); + await act(async () => { unmounted.resolve(agents); }); + expect(screen.queryByRole("dialog", { name: "Create room" })).not.toBeInTheDocument(); }); it("shows search-specific empty state copy", async () => { render(); await screen.findByRole("button", { name: /Alpha/i }); - await userEvent.type(screen.getByLabelText("Members"), "zzz"); + fireEvent.change(screen.getByLabelText("Members"), { target: { value: "zzz" } }); expect(screen.getByText("No agents match your search.")).toBeInTheDocument(); });