Address PR review feedback (#1362)
- Replace dynamic await import("@fusion/engine") in the agent-import route
with a static top-level import. The dynamic form is banned by the FN-3049
engine-import-regression test (bundler safety); my earlier reply mistook the
file's @fusion/core dynamic-import convention for a uniform rule — the
regression only forbids @fusion/engine. Verified the test now passes.
- AgentImportModal: capture and render dry-run `warnings` in the preview step
so the custom-role safeguard is shown BEFORE the import runs, not only after.
Adds a regression test for the preview warning.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -137,6 +137,7 @@ export function AgentImportModal({ isOpen, onClose, onImported, projectId, initi
|
||||
const [companyName, setCompanyName] = useState("Unknown");
|
||||
const [agents, setAgents] = useState<AgentPreview[]>([]);
|
||||
const [skills, setSkills] = useState<SkillPreview[]>([]);
|
||||
const [previewWarnings, setPreviewWarnings] = useState<string[]>([]);
|
||||
const [selectedAgentNames, setSelectedAgentNames] = useState<string[]>([]);
|
||||
const [selectedSkillNames, setSelectedSkillNames] = useState<string[]>([]);
|
||||
const [isParsing, setIsParsing] = useState(false);
|
||||
@@ -215,6 +216,7 @@ export function AgentImportModal({ isOpen, onClose, onImported, projectId, initi
|
||||
setCompanyName("Unknown");
|
||||
setAgents([]);
|
||||
setSkills([]);
|
||||
setPreviewWarnings([]);
|
||||
setSelectedAgentNames([]);
|
||||
setSelectedSkillNames([]);
|
||||
setIsParsing(false);
|
||||
@@ -345,6 +347,7 @@ export function AgentImportModal({ isOpen, onClose, onImported, projectId, initi
|
||||
created: string[];
|
||||
skipped: string[];
|
||||
errors: Array<{ name: string; error: string }>;
|
||||
warnings?: string[];
|
||||
};
|
||||
|
||||
const previewAgents = (data.agents && data.agents.length > 0)
|
||||
@@ -355,6 +358,7 @@ export function AgentImportModal({ isOpen, onClose, onImported, projectId, initi
|
||||
setCompanyName(data.companyName ?? "Unknown");
|
||||
setAgents(previewAgents);
|
||||
setSkills(previewSkills);
|
||||
setPreviewWarnings(Array.isArray(data.warnings) ? data.warnings : []);
|
||||
setSelectedAgentNames(previewAgents.map((agent) => agent.name));
|
||||
setSelectedSkillNames(previewSkills.map((skill) => skill.name));
|
||||
setStep("preview");
|
||||
@@ -678,6 +682,17 @@ export function AgentImportModal({ isOpen, onClose, onImported, projectId, initi
|
||||
<span className="agent-import-company-name">{companyName}</span>
|
||||
</div>
|
||||
|
||||
{previewWarnings.length > 0 && (
|
||||
<div className="agent-import-result-warnings">
|
||||
{previewWarnings.map((warning, idx) => (
|
||||
<div key={idx} className="agent-import-result-warning">
|
||||
<AlertTriangle size={12} />
|
||||
<span>{warning}</span>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
|
||||
<div className="agent-import-count">
|
||||
<FileText size={14} />
|
||||
<span>{agents.length} agent{agents.length !== 1 ? "s" : ""} found</span>
|
||||
|
||||
@@ -110,6 +110,36 @@ describe("AgentImportModal", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("surfaces dry-run warnings in the preview step, before import (issue #1261)", async () => {
|
||||
vi.mocked(globalThis.fetch).mockImplementationOnce(() => mockFetchResponse({
|
||||
ok: true,
|
||||
status: 200,
|
||||
body: {
|
||||
dryRun: true,
|
||||
companyName: "Acme Co",
|
||||
agents: [{ name: "CEO", role: "custom" }],
|
||||
created: ["CEO"],
|
||||
skipped: [],
|
||||
errors: [],
|
||||
warnings: [
|
||||
"1 imported agent(s) have role \"custom\" and won't be auto-assigned mission or queue work.",
|
||||
],
|
||||
},
|
||||
}));
|
||||
|
||||
render(<AgentImportModal isOpen={true} onClose={onClose} onImported={onImported} />);
|
||||
|
||||
fireEvent.change(screen.getByLabelText("Manifest content"), {
|
||||
target: { value: "---\nname: CEO\n---\nLead" },
|
||||
});
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: "Preview" }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText(/won't be auto-assigned mission or queue work/)).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
it("imports agents from preview step and shows result summary", async () => {
|
||||
vi.mocked(globalThis.fetch)
|
||||
.mockImplementationOnce(() => mockFetchResponse({
|
||||
|
||||
@@ -4,6 +4,7 @@ import { tmpdir } from "node:os";
|
||||
import { join, resolve } from "node:path";
|
||||
import { Readable } from "node:stream";
|
||||
import { pipeline as streamPipeline } from "node:stream/promises";
|
||||
import { listEligibleExecutorAgents } from "@fusion/engine";
|
||||
import { ApiError, badRequest, notFound, rateLimited } from "../api-error.js";
|
||||
import { createSessionDiagnostics } from "../ai-session-diagnostics.js";
|
||||
import { writeSSEEvent } from "../sse-buffer.js";
|
||||
@@ -741,7 +742,6 @@ async function persistImportedSkills(
|
||||
const customRoleCount = importItems.filter((item) => item.input.role === "custom").length;
|
||||
const importsAnExecutor = importItems.some((item) => item.input.role === "executor");
|
||||
if (customRoleCount > 0 && !importsAnExecutor) {
|
||||
const { listEligibleExecutorAgents } = await import("@fusion/engine");
|
||||
const existingExecutors = await listEligibleExecutorAgents(agentStore).catch(() => []);
|
||||
if (existingExecutors.length === 0) {
|
||||
importWarnings.push(
|
||||
|
||||
Reference in New Issue
Block a user