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) <noreply@runfusion.ai>
2026-08-16 01:50:38 -07:00
parent 7527d2651f
commit 3272affbb3
4 changed files with 165 additions and 35 deletions
--- a/.changeset/fn-9120-create-room-picker.md
+++ 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.
--- 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.
--- 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<Agent[]>([]);
  const [search, setSearch] = useState("");
  const [selectedAgentIds, setSelectedAgentIds] = useState<string[]>([]);
-  const [loadingAgents, setLoadingAgents] = useState(false);
+  const [agentLoadPhase, setAgentLoadPhase] = useState<"idle" | "loading" | "loaded" | "failed">("idle");
  const [submitError, setSubmitError] = useState<string | null>(null);
  const [isSubmitting, setIsSubmitting] = useState(false);
  const nameInputRef = useRef<HTMLInputElement>(null);
  const previousFocusRef = useRef<HTMLElement | null>(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.
        */}
        <div className="create-room-modal-member-list" data-testid="create-room-member-list">
-          {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" ? (
            <div className="create-room-modal-empty"><LoadingSpinner label={t("createRoom.loadingAgents", "Loading agents...")} /></div>
+          ) : agentLoadPhase === "failed" ? (
+            <div className="create-room-modal-empty">{t("createRoom.failedLoadAgents", "Failed to load agents.")}</div>
          ) : filteredAgents.length === 0 ? (
            <div className="create-room-modal-empty">
              {agents.length === 0 ? t("createRoom.noAgents", "No agents in this project yet.") : t("createRoom.noMatch", "No agents match your search.")}
--- 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<T>() {
+  let resolve!: (value: T) => void;
+  let reject!: (reason?: unknown) => void;
+  const promise = new Promise<T>((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(<CreateRoomModal isOpen onClose={vi.fn()} onCreate={vi.fn()} />);
-    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<any[]>();
+    const empty = createDeferred<any[]>();
+    const populated = createDeferred<any[]>();
+    mockFetchAgents
+      .mockImplementationOnce(() => loading.promise)
+      .mockImplementationOnce(() => empty.promise)
+      .mockImplementationOnce(() => populated.promise);
+    const { rerender } = render(<CreateRoomModal isOpen onClose={vi.fn()} onCreate={vi.fn()} />);

-    mockFetchAgents.mockResolvedValueOnce([]);
-    const empty = render(<CreateRoomModal isOpen onClose={vi.fn()} onCreate={vi.fn()} />);
-    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(<CreateRoomModal isOpen onClose={vi.fn()} onCreate={vi.fn()} />);
-    await screen.findByRole("button", { name: /Alpha/i });
-    await userEvent.type(screen.getByLabelText("Members"), "zzz");
+    rerender(<CreateRoomModal isOpen={false} onClose={vi.fn()} onCreate={vi.fn()} />);
+    rerender(<CreateRoomModal isOpen onClose={vi.fn()} onCreate={vi.fn()} />);
+    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(<CreateRoomModal isOpen={false} onClose={vi.fn()} onCreate={vi.fn()} />);
+    rerender(<CreateRoomModal isOpen onClose={vi.fn()} onCreate={vi.fn()} />);
+    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<any[]>();
+    const projectB = createDeferred<any[]>();
+    mockFetchAgents.mockImplementationOnce(() => projectA.promise).mockImplementationOnce(() => projectB.promise);
+    const { rerender } = render(<CreateRoomModal isOpen projectId="project-a" onClose={vi.fn()} onCreate={vi.fn()} />);
+    rerender(<CreateRoomModal isOpen={false} projectId="project-a" onClose={vi.fn()} onCreate={vi.fn()} />);
+    rerender(<CreateRoomModal isOpen projectId="project-b" onClose={vi.fn()} onCreate={vi.fn()} />);
+    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<any[]>();
+    const reloaded = createDeferred<any[]>();
+    const duplicateNameAgents = [agents[0], { ...agents[0], id: "agent-3" }];
+    mockFetchAgents.mockImplementationOnce(() => initial.promise).mockImplementationOnce(() => reloaded.promise);
+    const { rerender } = render(<CreateRoomModal isOpen projectId="project-a" onClose={vi.fn()} onCreate={vi.fn()} />);
+    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(<CreateRoomModal isOpen projectId="project-b" onClose={vi.fn()} onCreate={vi.fn()} />);
+    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<any[]>();
+    mockFetchAgents.mockImplementationOnce(() => rejected.promise);
+    const { unmount } = render(<CreateRoomModal isOpen onClose={vi.fn()} onCreate={vi.fn()} />);
+    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<any[]>();
+    mockFetchAgents.mockImplementationOnce(() => unmounted.promise);
+    unmount();
+    const second = render(<CreateRoomModal isOpen onClose={vi.fn()} onCreate={vi.fn()} />);
+    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(<CreateRoomModal isOpen onClose={vi.fn()} onCreate={vi.fn()} />);

    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();
  });