fix(issues): stop cold-load render loop on the Issues route (MUL-4985) (#5643)

Board and Swimlane crashed with "Maximum update depth exceeded" on first
paint of the Issues route. During cold load the member/agent/squad
directory queries are unresolved, so useActorName's `= []` defaults
allocated a fresh array every render and its `getActorName` memo changed
identity each render. BoardView's `groups` (and SwimLaneView's
`laneGroups`) list `getActorName` as a dep, so they churned every render
and re-fired the column-resync effect without end; react-virtuoso
escalated the loop into the reported crash. This predates MUL-4797 — it
was exposed when board/swimlane columns were virtualized with a real
<Virtuoso>.

- Share stable empty references for the loading directory snapshot so
  getActorName is referentially stable across cold-load renders (root
  cause).
- Equality-guard the shared column setter in useDragSettle so a
  content-equal rebuild returns the previous reference and cannot spin
  the resync effect (defense-in-depth).

Regression coverage: useActorName stability during cold load; the
column-setter equality guard; and Board (40 cards) + Swimlane rendered
with the REAL react-virtuoso under pending directories, which reproduce
"Maximum update depth exceeded" without the fix and pass with it.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
Naiyuan Qing
2026-07-20 08:52:02 +08:00
committed by GitHub
parent 002ea0d879
commit fcb4798f12
5 changed files with 515 additions and 5 deletions

View File

@@ -0,0 +1,98 @@
/**
* @vitest-environment jsdom
*/
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { renderHook } from "@testing-library/react";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import type { ReactNode } from "react";
import { setApiInstance } from "../api";
import type { ApiClient } from "../api/client";
import { workspaceKeys } from "./queries";
import { useActorName } from "./hooks";
// useActorName reads the current workspace from the core WorkspaceId provider;
// the directory-name resolution under test does not depend on the real id.
vi.mock("../hooks", () => ({
useWorkspaceId: () => "ws-1",
}));
function createWrapper(qc: QueryClient) {
return function Wrapper({ children }: { children: ReactNode }) {
return <QueryClientProvider client={qc}>{children}</QueryClientProvider>;
};
}
describe("useActorName", () => {
let qc: QueryClient;
beforeEach(() => {
qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
});
afterEach(() => {
qc.clear();
vi.restoreAllMocks();
});
// MUL-4985 regression: while the member/agent/squad directory queries are
// still loading, `data` is undefined. A `= []` default allocated a fresh
// array every render, so `getActorName` (memoized on those arrays) changed
// identity on every render. Consumers that list `getActorName` in their own
// memo deps (BoardView's `groups`, SwimLaneView's `laneGroups`) then churned
// a new value each render and spun the column-resync effect without end —
// an infinite re-render that react-virtuoso escalated into "Maximum update
// depth exceeded" on the Issues route. The fix shares one stable empty
// reference for the loading snapshot, so `getActorName` must be stable
// across re-renders while the directories are unresolved.
it("returns a referentially stable getActorName across renders during cold load", () => {
// Directory endpoints never resolve → the queries stay pending, so the
// hook renders repeatedly with undefined directory data (the cold-load
// state that used to loop).
const pending = () => new Promise<never>(() => {});
setApiInstance({
listMembers: pending,
listAgents: pending,
listSquads: pending,
} as unknown as ApiClient);
const { result, rerender } = renderHook(() => useActorName(), {
wrapper: createWrapper(qc),
});
const first = result.current.getActorName;
rerender();
const second = result.current.getActorName;
rerender();
const third = result.current.getActorName;
expect(second).toBe(first);
expect(third).toBe(first);
// A stable resolver over an empty directory still resolves gracefully.
expect(first("member", "user-1")).toBe("Unknown");
});
it("resolves names once the directories are loaded", () => {
// Seed the caches directly so the hook reads resolved directories on its
// first render — this guards that stabilizing the loading default did not
// break name resolution when data IS present.
const members = [{ user_id: "user-1", name: "Ada", avatar_url: null }];
const agents = [{ id: "agent-1", name: "Walt", avatar_url: null }];
const squads = [{ id: "squad-1", name: "Core", avatar_url: null }];
setApiInstance({
listMembers: () => Promise.resolve(members),
listAgents: () => Promise.resolve(agents),
listSquads: () => Promise.resolve(squads),
} as unknown as ApiClient);
qc.setQueryData(workspaceKeys.members("ws-1"), members);
qc.setQueryData(workspaceKeys.agents("ws-1"), agents);
qc.setQueryData(workspaceKeys.squads("ws-1"), squads);
const { result } = renderHook(() => useActorName(), {
wrapper: createWrapper(qc),
});
expect(result.current.getActorName("member", "user-1")).toBe("Ada");
expect(result.current.getActorName("agent", "agent-1")).toBe("Walt");
expect(result.current.getActorName("squad", "squad-1")).toBe("Core");
});
});

View File

@@ -2,10 +2,25 @@
import { useCallback, useMemo } from "react";
import { useQuery } from "@tanstack/react-query";
import type { Agent, MemberWithUser, Squad } from "../types";
import { useWorkspaceId } from "../hooks";
import { memberListOptions, agentListOptions, squadListOptions } from "./queries";
import { resolvePublicFileUrl } from "./avatar-url";
// Stable empties for the still-loading directory queries. A fresh `= []`
// default allocates a new array on every render while `data` is undefined,
// which makes `useMemo(..., [members, agents, squads])` recompute
// `getActorName` on every render during cold load. Consumers that list
// `getActorName` in their own memo deps (BoardView's `groups`, SwimLaneView's
// `laneGroups`) then churn a fresh value each render, and the board/list
// column resync `useEffect(setColumns, [groups])` re-fires without end — an
// infinite re-render that react-virtuoso turns into "Maximum update depth
// exceeded" on the Issues route (MUL-4985). Sharing one reference keeps the
// loading snapshot referentially stable.
const EMPTY_MEMBERS: MemberWithUser[] = [];
const EMPTY_AGENTS: Agent[] = [];
const EMPTY_SQUADS: Squad[] = [];
/**
* Pure actor-name resolution over explicit directory snapshots. Async flows
* (e.g. CSV export) must resolve names from directories they have awaited
@@ -32,9 +47,9 @@ export function buildActorNameResolver(directories: {
export function useActorName() {
const wsId = useWorkspaceId();
const { data: members = [] } = useQuery(memberListOptions(wsId));
const { data: agents = [] } = useQuery(agentListOptions(wsId));
const { data: squads = [] } = useQuery(squadListOptions(wsId));
const { data: members = EMPTY_MEMBERS } = useQuery(memberListOptions(wsId));
const { data: agents = EMPTY_AGENTS } = useQuery(agentListOptions(wsId));
const { data: squads = EMPTY_SQUADS } = useQuery(squadListOptions(wsId));
const getMemberName = useCallback((userId: string) => {
const m = members.find((m) => m.user_id === userId);

View File

@@ -0,0 +1,290 @@
/**
* @vitest-environment jsdom
*
* MUL-4985 regression — cold-load render loop on the Issues route.
*
* These tests render Board and Swimlane with the REAL react-virtuoso and the
* REAL `useActorName`, while the member/agent/squad directory queries are held
* pending (the cold-load state). Before the fix, `useActorName` returned a
* fresh `getActorName` on every render, which churned BoardView's `groups` /
* SwimLaneView's `laneGroups`, re-fired the column-resync effect without end,
* and react-virtuoso escalated it into "Maximum update depth exceeded". A
* looping render never settles, so each test would hang/throw; the fix lets it
* paint. (Unlike the sibling swimlane-view.test.tsx, this file intentionally
* does NOT mock react-virtuoso or useActorName — those two reals are the whole
* point of the reproduction.)
*/
import { describe, it, expect, vi, beforeEach } from "vitest";
import { render, screen, waitFor } from "@testing-library/react";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import type { ReactNode } from "react";
import { BoardView } from "./board-view";
import { SwimLaneView } from "./swimlane-view";
import { IssueContextMenuProvider } from "../actions";
import { setApiInstance } from "@multica/core/api";
import type { ApiClient } from "@multica/core/api/client";
import type { Issue } from "@multica/core/types";
import { I18nProvider } from "@multica/core/i18n/react";
import enCommon from "../../locales/en/common.json";
import enIssues from "../../locales/en/issues.json";
const TEST_RESOURCES = { en: { common: enCommon, issues: enIssues } };
vi.mock("@multica/core/hooks", () => ({
useWorkspaceId: () => "ws-1",
}));
vi.mock("@multica/core/paths", async () => {
const actual = await vi.importActual<typeof import("@multica/core/paths")>(
"@multica/core/paths",
);
return {
...actual,
useWorkspaceSlug: () => "acme",
useRequiredWorkspaceSlug: () => "acme",
useWorkspacePaths: () => actual.paths.workspace("acme"),
};
});
const mockAuthUser = { id: "user-1", email: "test@test.com", name: "Test User" };
vi.mock("@multica/core/auth", () => ({
useAuthStore: Object.assign(
(selector?: any) => {
const state = { user: mockAuthUser, isAuthenticated: true };
return selector ? selector(state) : state;
},
{ getState: () => ({ user: mockAuthUser, isAuthenticated: true }) },
),
registerAuthStore: vi.fn(),
createAuthStore: vi.fn(),
}));
vi.mock("../../navigation", () => ({
AppLink: ({ children, href, ...props }: any) => (
<a href={href} {...props}>
{children}
</a>
),
useNavigation: () => ({ push: vi.fn(), pathname: "/issues" }),
NavigationProvider: ({ children }: { children: React.ReactNode }) => children,
}));
vi.mock("@multica/core/issues/config", () => ({
ALL_STATUSES: ["backlog", "todo", "in_progress", "in_review", "done", "blocked", "cancelled"],
STATUS_ORDER: ["backlog", "todo", "in_progress", "in_review", "done", "blocked", "cancelled"],
STATUS_CONFIG: {
backlog: { label: "Backlog", iconColor: "text-muted-foreground", hoverBg: "hover:bg-accent" },
todo: { label: "Todo", iconColor: "text-muted-foreground", hoverBg: "hover:bg-accent" },
in_progress: { label: "In Progress", iconColor: "text-warning", hoverBg: "hover:bg-warning/10" },
in_review: { label: "In Review", iconColor: "text-success", hoverBg: "hover:bg-success/10" },
done: { label: "Done", iconColor: "text-info", hoverBg: "hover:bg-info/10" },
blocked: { label: "Blocked", iconColor: "text-destructive", hoverBg: "hover:bg-destructive/10" },
cancelled: { label: "Cancelled", iconColor: "text-muted-foreground", hoverBg: "hover:bg-accent" },
},
PRIORITY_ORDER: ["urgent", "high", "medium", "low", "none"],
PRIORITY_CONFIG: {
urgent: { label: "Urgent", bars: 4, color: "text-destructive" },
high: { label: "High", bars: 3, color: "text-warning" },
medium: { label: "Medium", bars: 2, color: "text-warning" },
low: { label: "Low", bars: 1, color: "text-info" },
none: { label: "No priority", bars: 0, color: "text-muted-foreground" },
},
}));
const mockLoadMore = vi.fn();
const loadMoreResult = {
total: 0,
loaded: 0,
hasMore: false,
isLoading: false,
loadMore: mockLoadMore,
};
vi.mock("@multica/core/issues/mutations", async (importOriginal) => {
const actual = await importOriginal<typeof import("@multica/core/issues/mutations")>();
return {
...actual,
useLoadMoreByStatus: () => loadMoreResult,
useLoadMoreByAssigneeGroup: () => loadMoreResult,
};
});
vi.mock("@multica/core/properties", async (importOriginal) => {
const actual = await importOriginal<typeof import("@multica/core/properties")>();
return {
...actual,
useSetIssueProperty: () => ({ mutate: vi.fn(), mutateAsync: vi.fn() }),
useUnsetIssueProperty: () => ({ mutate: vi.fn(), mutateAsync: vi.fn() }),
};
});
// Board default grouping is "status"; swimlane switches to "assignee" per test.
const mockViewState: Record<string, unknown> = {
grouping: "status",
sortBy: "position",
sortDirection: "asc",
cardProperties: { priority: true, assignee: true, dueDate: true, project: true, childProgress: true, labels: true },
swimlaneGrouping: "assignee",
swimlaneOrders: { parent: [], project: [], assignee: [] },
collapsedSwimlanes: { parent: [], project: [], assignee: [] },
setSwimlaneGrouping: vi.fn(),
setSwimlaneOrder: vi.fn(),
toggleSwimlaneCollapsed: vi.fn(),
hideStatus: vi.fn(),
showStatus: vi.fn(),
priorityFilters: [],
assigneeFilters: [],
includeNoAssignee: false,
creatorFilters: [],
projectFilters: [],
includeNoProject: false,
labelFilters: [],
propertyFilters: {},
cardPropertyIds: [],
agentRunningFilter: false,
};
vi.mock("@multica/core/issues/stores/view-store-context", () => ({
ViewStoreProvider: ({ children }: { children: ReactNode }) => children,
useViewStore: (selector?: any) => (selector ? selector(mockViewState) : mockViewState),
useViewStoreApi: () => ({ getState: () => mockViewState, setState: vi.fn(), subscribe: vi.fn() }),
}));
vi.mock("@multica/core/modals", () => ({
useModalStore: Object.assign(
() => ({ open: vi.fn() }),
{ getState: () => ({ open: vi.fn() }) },
),
}));
vi.mock("@dnd-kit/core", () => ({
DndContext: ({ children }: any) => children,
DragOverlay: () => null,
PointerSensor: class {},
useSensor: () => ({}),
useSensors: () => [],
useDroppable: () => ({ setNodeRef: vi.fn(), isOver: false }),
pointerWithin: vi.fn(),
closestCenter: vi.fn(),
}));
vi.mock("@dnd-kit/sortable", () => ({
SortableContext: ({ children }: any) => children,
verticalListSortingStrategy: {},
arrayMove: <T,>(arr: T[]): T[] => arr.slice(),
useSortable: () => ({
attributes: {},
listeners: {},
setNodeRef: vi.fn(),
transform: null,
transition: null,
isDragging: false,
}),
}));
vi.mock("@dnd-kit/utilities", () => ({
CSS: { Transform: { toString: () => undefined } },
}));
// The whole point: directory queries stay pending so useActorName renders in
// the cold-load state. A never-resolving promise keeps `data` undefined.
const pending = () => new Promise<never>(() => {});
function makeIssue(overrides: Partial<Issue> & { id: string }): Issue {
return {
workspace_id: "ws-1",
number: 1,
identifier: `PROJ-${overrides.id}`,
title: `Issue ${overrides.id}`,
description: null,
status: "todo",
priority: "none",
assignee_type: null,
assignee_id: null,
creator_type: "member",
creator_id: "user-1",
parent_issue_id: null,
project_id: null,
position: 100,
stage: null,
start_date: null,
due_date: null,
metadata: {},
properties: {},
created_at: "2026-01-01T00:00:00Z",
updated_at: "2026-01-01T00:00:00Z",
...overrides,
};
}
function renderWithProviders(ui: ReactNode) {
const qc = new QueryClient({
defaultOptions: { queries: { retry: false, gcTime: 0 } },
});
return render(
<QueryClientProvider client={qc}>
<I18nProvider resources={TEST_RESOURCES} locale="en">
<IssueContextMenuProvider>{ui}</IssueContextMenuProvider>
</I18nProvider>
</QueryClientProvider>,
);
}
describe("Issues cold-load render loop (MUL-4985)", () => {
beforeEach(() => {
vi.clearAllMocks();
mockViewState.grouping = "status";
mockViewState.swimlaneGrouping = "assignee";
setApiInstance({
listMembers: pending,
listAgents: pending,
listSquads: pending,
getAgentTaskSnapshot: () => Promise.resolve([]),
listChildrenByParents: () => Promise.resolve({ issues: [] }),
listProjects: pending,
// ActorAvatar resolves image URLs against the API base.
getBaseUrl: () => "",
} as unknown as ApiClient);
});
it("Board with a large column paints during cold load (real Virtuoso mounts, no update-depth loop)", async () => {
// > BOARD_VIRTUALIZE_THRESHOLD (30) issues in one status column so a real
// <Virtuoso> mounts (VirtuosoSeed is used below the threshold and cannot
// reproduce the store-driven loop).
const issues = Array.from({ length: 40 }, (_, i) =>
makeIssue({ id: `b${i}`, title: `Board Card ${i}`, status: "todo", position: 100 + i }),
);
renderWithProviders(
<BoardView
issues={issues}
visibleStatuses={["todo", "in_progress", "done"]}
hiddenStatuses={[]}
onMoveIssue={vi.fn()}
/>,
);
// Reaching a stable paint (column header visible) proves the render settled
// instead of looping.
await waitFor(() => {
expect(screen.getByText("Todo")).toBeInTheDocument();
});
expect(screen.getByText("Board Card 0")).toBeInTheDocument();
});
it("Swimlane grouped by assignee paints during cold load (real Virtuoso mounts, no update-depth loop)", async () => {
mockViewState.swimlaneGrouping = "assignee";
const issues = [
makeIssue({ id: "s1", title: "Swim Card 1", assignee_type: "member", assignee_id: "user-1", status: "todo" }),
makeIssue({ id: "s2", title: "Swim Card 2", assignee_type: "agent", assignee_id: "agent-1", status: "in_progress" }),
makeIssue({ id: "s3", title: "Swim Card 3", assignee_type: null, assignee_id: null, status: "todo" }),
];
renderWithProviders(
<SwimLaneView issues={issues} onMoveIssue={vi.fn()} />,
);
await waitFor(() => {
expect(screen.getByText("Swim Card 1")).toBeInTheDocument();
});
expect(screen.getByText("Swim Card 3")).toBeInTheDocument();
});
});

View File

@@ -0,0 +1,53 @@
/**
* @vitest-environment jsdom
*/
import { describe, it, expect } from "vitest";
import { useEffect } from "react";
import { render, act } from "@testing-library/react";
import { useDragSettle } from "./use-drag-settle";
describe("useDragSettle", () => {
// MUL-4985 defense-in-depth: the board/list/swimlane resync effect calls
// `setColumns(buildColumns(...))` whenever its `groups` input changes
// identity. When `groups` churns a fresh-but-content-equal value every render
// (the cold-load failure mode), an unguarded setter allocated a new column
// object each render and spun into "Maximum update depth exceeded". The
// equality guard returns the previous reference for a content-equal rebuild,
// so React bails and the loop cannot start.
it("does not loop when a resync effect rebuilds a content-equal column map every render", () => {
let renders = 0;
function Harness() {
renders++;
const { columns, setColumns } = useDragSettle(() => ({ todo: ["a", "b"] }));
// A fresh object of identical content on every render — what an unstable
// `groups` fed through buildColumns produces.
useEffect(() => {
setColumns({ todo: ["a", "b"] });
});
return <div>{Object.keys(columns).join(",")}</div>;
}
expect(() => render(<Harness />)).not.toThrow();
// A guarded setter settles in one or two renders; an unguarded loop is in
// the hundreds before React throws. Bound generously to stay robust.
expect(renders).toBeLessThan(10);
});
it("still applies a content-changed column update", () => {
function Harness({ next }: { next: Record<string, string[]> }) {
const { columns, setColumns } = useDragSettle(() => ({ todo: ["a"] }));
useEffect(() => {
setColumns(next);
}, [next, setColumns]);
return <div data-testid="cols">{(columns.todo ?? []).join(",")}</div>;
}
const { getByTestId, rerender } = render(<Harness next={{ todo: ["a"] }} />);
expect(getByTestId("cols").textContent).toBe("a");
act(() => {
rerender(<Harness next={{ todo: ["a", "b"] }} />);
});
expect(getByTestId("cols").textContent).toBe("a,b");
});
});

View File

@@ -1,4 +1,36 @@
import { useCallback, useEffect, useRef, useState } from "react";
import {
useCallback,
useEffect,
useRef,
useState,
type Dispatch,
type SetStateAction,
} from "react";
/**
* Content equality for the column map (`columnId -> ordered issue ids`). Two
* maps are equal when they have the same column keys and each column's id list
* matches element-for-element. Used to skip no-op `setColumns` writes.
*/
function columnsEqual(
a: Record<string, string[]>,
b: Record<string, string[]>,
): boolean {
if (a === b) return true;
const aKeys = Object.keys(a);
const bKeys = Object.keys(b);
if (aKeys.length !== bKeys.length) return false;
for (const key of aKeys) {
const av = a[key];
const bv = b[key];
if (av === bv) continue;
if (!av || !bv || av.length !== bv.length) return false;
for (let i = 0; i < av.length; i++) {
if (av[i] !== bv[i]) return false;
}
}
return true;
}
/**
* Shared drag/settle state machine for the issue boards (board-view, list-view).
@@ -30,12 +62,34 @@ export function useDragSettle(
const recentlyMovedRef = useRef(false);
const [settleVersion, setSettleVersion] = useState(0);
const [columns, setColumns] = useState<Record<string, string[]>>(
const [columns, setColumnsState] = useState<Record<string, string[]>>(
initialColumns,
);
const columnsRef = useRef(columns);
columnsRef.current = columns;
// Equality-guarded column setter. When a resync (or a no-op drag) produces a
// column map whose contents match the current one, return the SAME reference
// so React bails out of the re-render. Second line of defense against the
// cold-load update loop (MUL-4985): even if some input to `buildColumns`
// regains a per-render-unstable identity, a content-equal rebuild no longer
// forces a new state object and cannot spin the resync effect. It never
// changes the resulting value — only skips redundant writes — so drag/settle
// semantics are unchanged.
const setColumns = useCallback<
Dispatch<SetStateAction<Record<string, string[]>>>
>((update) => {
setColumnsState((prev) => {
const next =
typeof update === "function"
? (update as (p: Record<string, string[]>) => Record<string, string[]>)(
prev,
)
: update;
return columnsEqual(prev, next) ? prev : next;
});
}, []);
useEffect(() => {
const id = requestAnimationFrame(() => {
recentlyMovedRef.current = false;