fix(FN-1299): stabilize agent detail refresh callbacks
- Stabilize AgentDetailView callback usage by storing onClose/addToast in refs and removing them from loadAgent dependencies - Prevent full-screen loading spinner on background refresh by only showing it before the initial agent load - Memoize detail modal handlers in AgentsView to keep callback identities stable across parent re-renders - Add regression tests covering callback identity churn and refresh behavior without loading-state flicker
This commit is contained in:
@@ -84,19 +84,30 @@ export function AgentDetailView({ agentId, projectId, onClose, addToast, onChild
|
||||
const [activeTab, setActiveTab] = useState<TabId>("dashboard");
|
||||
const [isStreaming, setIsStreaming] = useState(false);
|
||||
const logContainerRef = useRef<HTMLDivElement>(null);
|
||||
const onCloseRef = useRef(onClose);
|
||||
const addToastRef = useRef(addToast);
|
||||
const agentRef = useRef<AgentDetail | null>(null);
|
||||
|
||||
onCloseRef.current = onClose;
|
||||
addToastRef.current = addToast;
|
||||
agentRef.current = agent;
|
||||
|
||||
const loadAgent = useCallback(async () => {
|
||||
setIsLoading(true);
|
||||
const showLoadingSpinner = agentRef.current === null;
|
||||
if (showLoadingSpinner) {
|
||||
setIsLoading(true);
|
||||
}
|
||||
|
||||
try {
|
||||
const data = await fetchAgent(agentId, projectId);
|
||||
setAgent(data);
|
||||
} catch (err: any) {
|
||||
addToast(`Failed to load agent: ${err.message}`, "error");
|
||||
onClose();
|
||||
addToastRef.current(`Failed to load agent: ${err.message}`, "error");
|
||||
onCloseRef.current();
|
||||
} finally {
|
||||
setIsLoading(false);
|
||||
}
|
||||
}, [agentId, addToast, onClose, projectId]);
|
||||
}, [agentId, projectId]);
|
||||
|
||||
const loadLogs = useCallback(async () => {
|
||||
// Agent logs are tied to tasks, not agents directly.
|
||||
|
||||
@@ -343,6 +343,14 @@ export function AgentsView({ addToast, projectId }: AgentsViewProps) {
|
||||
}
|
||||
};
|
||||
|
||||
const handleCloseDetail = useCallback(() => {
|
||||
setSelectedAgentId(null);
|
||||
}, []);
|
||||
|
||||
const handleChildClick = useCallback((childId: string) => {
|
||||
setSelectedAgentId(childId);
|
||||
}, []);
|
||||
|
||||
const handleRunHeartbeat = async (agentId: string, agentName: string) => {
|
||||
try {
|
||||
await startAgentRun(agentId, projectId, { source: "on_demand", triggerDetail: "Triggered from dashboard" });
|
||||
@@ -942,9 +950,9 @@ export function AgentsView({ addToast, projectId }: AgentsViewProps) {
|
||||
<AgentDetailView
|
||||
agentId={selectedAgentId}
|
||||
projectId={projectId}
|
||||
onClose={() => setSelectedAgentId(null)}
|
||||
onClose={handleCloseDetail}
|
||||
addToast={addToast}
|
||||
onChildClick={(childId) => setSelectedAgentId(childId)}
|
||||
onChildClick={handleChildClick}
|
||||
/>
|
||||
)}
|
||||
|
||||
|
||||
@@ -272,6 +272,73 @@ describe("AgentDetailView", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("does not refetch or show loading spinner when onClose/addToast callback identities change", async () => {
|
||||
const initialOnClose = vi.fn();
|
||||
const initialAddToast = vi.fn();
|
||||
|
||||
const { rerender } = render(
|
||||
<AgentDetailView
|
||||
agentId="agent-001"
|
||||
onClose={initialOnClose}
|
||||
addToast={initialAddToast}
|
||||
/>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByRole("heading", { name: "Test Agent" })).toBeInTheDocument();
|
||||
});
|
||||
expect(mockFetchAgent).toHaveBeenCalledTimes(1);
|
||||
|
||||
rerender(
|
||||
<AgentDetailView
|
||||
agentId="agent-001"
|
||||
onClose={vi.fn()}
|
||||
addToast={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(mockFetchAgent).toHaveBeenCalledTimes(1);
|
||||
expect(screen.queryByText("Loading agent...")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("refreshes agent data without showing full-screen loading spinner after initial load", async () => {
|
||||
const user = userEvent.setup();
|
||||
let resolveRefresh: ((value: AgentDetail) => void) | undefined;
|
||||
|
||||
mockFetchAgent
|
||||
.mockImplementationOnce(async () => createMockAgent())
|
||||
.mockImplementationOnce(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
resolveRefresh = resolve;
|
||||
}),
|
||||
);
|
||||
|
||||
render(
|
||||
<AgentDetailView
|
||||
agentId="agent-001"
|
||||
onClose={vi.fn()}
|
||||
addToast={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByRole("heading", { name: "Test Agent" })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
await user.click(screen.getByTitle("Refresh"));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockFetchAgent).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
expect(screen.queryByText("Loading agent...")).not.toBeInTheDocument();
|
||||
|
||||
resolveRefresh?.(createMockAgent({ updatedAt: "2024-01-01T00:10:00.000Z" }));
|
||||
await waitFor(() => {
|
||||
expect(screen.getByRole("heading", { name: "Test Agent" })).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
it("displays role badge", async () => {
|
||||
render(
|
||||
<AgentDetailView
|
||||
|
||||
Reference in New Issue
Block a user