feat(FN-2386): replace reports-to ID input with manager selector
- Load available agents when the New Agent dialog opens and present them in a Reports To dropdown - Keep manager assignment optional with a default "No manager" option and graceful fallback when manager fetch fails - Show manager name plus ID in the step-3 summary while still submitting reportsTo as the selected manager ID - Add focused NewAgentDialog tests for manager option loading, payload behavior, summary rendering, and failure handling
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
import { useState, useEffect, useCallback } from "react";
|
||||
import type { AgentCapability, ModelInfo, AgentGenerationSpec } from "../api";
|
||||
import { createAgent, fetchModels, updateGlobalSettings } from "../api";
|
||||
import type { Agent, AgentCapability, ModelInfo, AgentGenerationSpec } from "../api";
|
||||
import { createAgent, fetchAgents, fetchModels, updateGlobalSettings } from "../api";
|
||||
import { CustomModelDropdown } from "./CustomModelDropdown";
|
||||
import { ProviderIcon } from "./ProviderIcon";
|
||||
import { AgentGenerationModal } from "./AgentGenerationModal";
|
||||
@@ -63,6 +63,10 @@ export function NewAgentDialog({ isOpen, onClose, onCreated, projectId }: NewAge
|
||||
const [favoriteProviders, setFavoriteProviders] = useState<string[]>([]);
|
||||
const [favoriteModels, setFavoriteModels] = useState<string[]>([]);
|
||||
|
||||
// Manager dropdown state
|
||||
const [availableManagers, setAvailableManagers] = useState<Agent[]>([]);
|
||||
const [managersLoading, setManagersLoading] = useState(false);
|
||||
|
||||
// Load models when dialog opens — guard prevents async setState after test assertions
|
||||
useEffect(() => {
|
||||
if (!isOpen) return;
|
||||
@@ -79,6 +83,22 @@ export function NewAgentDialog({ isOpen, onClose, onCreated, projectId }: NewAge
|
||||
.finally(() => setModelsLoading(false));
|
||||
}, [isOpen]);
|
||||
|
||||
// Load manager options when dialog opens
|
||||
useEffect(() => {
|
||||
if (!isOpen) return;
|
||||
setManagersLoading(true);
|
||||
setAvailableManagers([]);
|
||||
fetchAgents(undefined, projectId)
|
||||
.then((agents) => {
|
||||
setAvailableManagers(agents);
|
||||
})
|
||||
.catch(() => {
|
||||
// Gracefully handle — manager selector will show "No manager" only
|
||||
setAvailableManagers([]);
|
||||
})
|
||||
.finally(() => setManagersLoading(false));
|
||||
}, [isOpen, projectId]);
|
||||
|
||||
// Selected model in "provider/modelId" format, or "" for default
|
||||
const selectedModel = runtimeConfig.model.includes("/")
|
||||
? runtimeConfig.model
|
||||
@@ -210,6 +230,10 @@ export function NewAgentDialog({ isOpen, onClose, onCreated, projectId }: NewAge
|
||||
};
|
||||
|
||||
const selectedRole = AGENT_ROLES.find(r => r.value === role);
|
||||
const selectedReportsToId = reportsTo.trim();
|
||||
const selectedManager = selectedReportsToId
|
||||
? availableManagers.find((manager) => manager.id === selectedReportsToId)
|
||||
: undefined;
|
||||
|
||||
return (
|
||||
<div className="agent-dialog-overlay" onClick={(e) => { if (e.target === e.currentTarget) handleClose(); }}>
|
||||
@@ -321,15 +345,21 @@ export function NewAgentDialog({ isOpen, onClose, onCreated, projectId }: NewAge
|
||||
<div className="agent-dialog-section">
|
||||
<div className="agent-dialog-section-header">Configuration</div>
|
||||
<div className="agent-dialog-field">
|
||||
<label htmlFor="agent-reports-to">Reports To <span className="agent-dialog-optional">(optional agent ID)</span></label>
|
||||
<input
|
||||
<label htmlFor="agent-reports-to">Reports To <span className="agent-dialog-optional">(optional)</span></label>
|
||||
<select
|
||||
id="agent-reports-to"
|
||||
type="text"
|
||||
className="input"
|
||||
placeholder="e.g. agent-1234abcd"
|
||||
className="select"
|
||||
value={reportsTo}
|
||||
onChange={e => setReportsTo(e.target.value)}
|
||||
/>
|
||||
disabled={managersLoading}
|
||||
>
|
||||
<option value="">No manager</option>
|
||||
{availableManagers.map((manager) => (
|
||||
<option key={manager.id} value={manager.id}>
|
||||
{manager.name} ({manager.id})
|
||||
</option>
|
||||
))}
|
||||
</select>
|
||||
</div>
|
||||
<div className="agent-dialog-field">
|
||||
<label htmlFor="agent-soul">Soul <span className="agent-dialog-optional">(optional)</span></label>
|
||||
@@ -479,10 +509,14 @@ export function NewAgentDialog({ isOpen, onClose, onCreated, projectId }: NewAge
|
||||
<span className="agent-dialog-summary-row-label">Role</span>
|
||||
<span>{selectedRole?.icon} {selectedRole?.label}</span>
|
||||
</div>
|
||||
{reportsTo.trim() && (
|
||||
{selectedReportsToId && (
|
||||
<div className="agent-dialog-summary-row">
|
||||
<span className="agent-dialog-summary-row-label">Reports To</span>
|
||||
<span>{reportsTo.trim()}</span>
|
||||
<span>
|
||||
{selectedManager
|
||||
? `${selectedManager.name} (${selectedManager.id})`
|
||||
: selectedReportsToId}
|
||||
</span>
|
||||
</div>
|
||||
)}
|
||||
{instructionsPath.trim() && (
|
||||
|
||||
@@ -7,6 +7,7 @@ import * as apiModule from "../../api";
|
||||
// Mock the API module
|
||||
vi.mock("../../api", () => ({
|
||||
createAgent: vi.fn(),
|
||||
fetchAgents: vi.fn(),
|
||||
fetchModels: vi.fn(),
|
||||
updateGlobalSettings: vi.fn(),
|
||||
fetchDiscoveredSkills: vi.fn(),
|
||||
@@ -75,6 +76,7 @@ vi.mock("../AgentGenerationModal", () => ({
|
||||
}));
|
||||
|
||||
const mockCreateAgent = vi.mocked(apiModule.createAgent);
|
||||
const mockFetchAgents = vi.mocked(apiModule.fetchAgents);
|
||||
const mockFetchModels = vi.mocked(apiModule.fetchModels);
|
||||
const mockUpdateGlobalSettings = vi.mocked(apiModule.updateGlobalSettings);
|
||||
const mockFetchDiscoveredSkills = vi.mocked(apiModule.fetchDiscoveredSkills);
|
||||
@@ -93,6 +95,27 @@ const MOCK_SKILLS_RESPONSE = [
|
||||
{ id: "skill-2", name: "Skill Two", path: "/path/skill-2", relativePath: "skills/skill-2", enabled: true, metadata: { source: "*", scope: "user" as const, origin: "top-level" as const } },
|
||||
];
|
||||
|
||||
const MOCK_MANAGER_AGENTS = [
|
||||
{
|
||||
id: "agent-manager-1",
|
||||
name: "Manager One",
|
||||
role: "executor",
|
||||
state: "idle",
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
metadata: {},
|
||||
},
|
||||
{
|
||||
id: "agent-manager-2",
|
||||
name: "Manager Two",
|
||||
role: "reviewer",
|
||||
state: "active",
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
metadata: {},
|
||||
},
|
||||
];
|
||||
|
||||
async function openModelDropdown(label = "Model") {
|
||||
fireEvent.click(screen.getByRole("button", { name: label }));
|
||||
|
||||
@@ -118,6 +141,7 @@ describe("NewAgentDialog", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockFetchModels.mockResolvedValue(MOCK_MODELS_RESPONSE);
|
||||
mockFetchAgents.mockResolvedValue(MOCK_MANAGER_AGENTS as any);
|
||||
mockCreateAgent.mockResolvedValue({} as any);
|
||||
mockUpdateGlobalSettings.mockResolvedValue({});
|
||||
mockFetchDiscoveredSkills.mockResolvedValue(MOCK_SKILLS_RESPONSE);
|
||||
@@ -154,6 +178,114 @@ describe("NewAgentDialog", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("manager dropdown", () => {
|
||||
it("fetches manager options on open with projectId", async () => {
|
||||
render(
|
||||
<NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} projectId="proj-123" />,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockFetchAgents).toHaveBeenCalledWith(undefined, "proj-123");
|
||||
});
|
||||
});
|
||||
|
||||
it("renders reports-to as a select with no-manager and fetched manager options", async () => {
|
||||
render(
|
||||
<NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockFetchAgents).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
const reportsToSelect = screen.getByLabelText(/Reports To/) as HTMLSelectElement;
|
||||
expect(reportsToSelect.tagName).toBe("SELECT");
|
||||
expect(within(reportsToSelect).getByRole("option", { name: "No manager" })).toBeTruthy();
|
||||
expect(within(reportsToSelect).getByRole("option", { name: "Manager One (agent-manager-1)" })).toBeTruthy();
|
||||
expect(within(reportsToSelect).getByRole("option", { name: "Manager Two (agent-manager-2)" })).toBeTruthy();
|
||||
});
|
||||
|
||||
it("sends selected manager id as reportsTo in createAgent payload", async () => {
|
||||
const user = userEvent.setup();
|
||||
render(
|
||||
<NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />,
|
||||
);
|
||||
|
||||
await waitFor(() => expect(mockFetchAgents).toHaveBeenCalledOnce());
|
||||
|
||||
const nameInput = screen.getByLabelText(/Name/);
|
||||
await user.type(nameInput, "Agent With Manager");
|
||||
|
||||
const reportsToSelect = screen.getByLabelText(/Reports To/);
|
||||
await user.selectOptions(reportsToSelect, "agent-manager-1");
|
||||
|
||||
await user.click(screen.getByText("Next"));
|
||||
await user.click(screen.getByText("Next"));
|
||||
|
||||
expect(screen.getByText("Reports To")).toBeTruthy();
|
||||
expect(screen.getByText("Manager One (agent-manager-1)")).toBeTruthy();
|
||||
|
||||
await user.click(screen.getByText("Create"));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockCreateAgent).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
expect(mockCreateAgent.mock.calls[0][0]).toMatchObject({
|
||||
name: "Agent With Manager",
|
||||
reportsTo: "agent-manager-1",
|
||||
});
|
||||
});
|
||||
|
||||
it("omits reportsTo from payload when no manager is selected", async () => {
|
||||
const user = userEvent.setup();
|
||||
render(
|
||||
<NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />,
|
||||
);
|
||||
|
||||
await waitFor(() => expect(mockFetchAgents).toHaveBeenCalledOnce());
|
||||
|
||||
await user.type(screen.getByLabelText(/Name/), "Agent Without Manager");
|
||||
await user.click(screen.getByText("Next"));
|
||||
await user.click(screen.getByText("Next"));
|
||||
await user.click(screen.getByText("Create"));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockCreateAgent).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
expect(mockCreateAgent.mock.calls[0][0].reportsTo).toBeUndefined();
|
||||
});
|
||||
|
||||
it("keeps dialog functional when manager fetch fails", async () => {
|
||||
mockFetchAgents.mockRejectedValueOnce(new Error("manager fetch failed"));
|
||||
const user = userEvent.setup();
|
||||
|
||||
render(
|
||||
<NewAgentDialog isOpen={true} onClose={mockOnClose} onCreated={mockOnCreated} />,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockFetchAgents).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
const reportsToSelect = screen.getByLabelText(/Reports To/) as HTMLSelectElement;
|
||||
expect(within(reportsToSelect).getByRole("option", { name: "No manager" })).toBeTruthy();
|
||||
expect(reportsToSelect.options).toHaveLength(1);
|
||||
|
||||
await user.type(screen.getByLabelText(/Name/), "Agent Works Without Managers");
|
||||
await user.click(screen.getByText("Next"));
|
||||
await user.click(screen.getByText("Next"));
|
||||
await user.click(screen.getByText("Create"));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockCreateAgent).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
expect(mockCreateAgent.mock.calls[0][0].reportsTo).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("model dropdown", () => {
|
||||
it("fetches models on mount", async () => {
|
||||
await act(async () => {
|
||||
|
||||
Reference in New Issue
Block a user