feat(FN-3324): add runtime guard regression sentinel
Adds a regression test sentinel in the engine's in-process runtime test suite to guard against runtime guard behavior regressions (FN-3324). Fusion-Task-Id: FN-3324
This commit is contained in:
@@ -137,6 +137,64 @@ describe("AgentStore", () => {
|
||||
// ── createAgent ───────────────────────────────────────────────────
|
||||
|
||||
describe("createAgent", () => {
|
||||
describe("createAgent name uniqueness", () => {
|
||||
it("rejects creating a non-ephemeral agent with a duplicate name", async () => {
|
||||
const created = await store.createAgent({
|
||||
name: "Alpha",
|
||||
role: "executor",
|
||||
});
|
||||
|
||||
await expect(
|
||||
store.createAgent({
|
||||
name: "Alpha",
|
||||
role: "reviewer",
|
||||
}),
|
||||
).rejects.toThrow(`Agent with name "Alpha" already exists (agentId: ${created.id})`);
|
||||
});
|
||||
|
||||
it("allows creating ephemeral agents with duplicate names", async () => {
|
||||
const first = await store.createAgent({
|
||||
name: "executor-FN-123",
|
||||
role: "executor",
|
||||
metadata: { agentKind: "task-worker", taskWorker: true },
|
||||
});
|
||||
|
||||
const second = await store.createAgent({
|
||||
name: "executor-FN-123",
|
||||
role: "executor",
|
||||
metadata: { agentKind: "task-worker", taskWorker: true },
|
||||
});
|
||||
|
||||
expect(first.id).not.toBe(second.id);
|
||||
expect(first.name).toBe(second.name);
|
||||
});
|
||||
|
||||
it("findAgentByName returns the correct agent", async () => {
|
||||
const created = await store.createAgent({
|
||||
name: "Beta",
|
||||
role: "executor",
|
||||
});
|
||||
|
||||
const found = await store.findAgentByName("Beta");
|
||||
const missing = await store.findAgentByName("Gamma");
|
||||
|
||||
expect(found?.id).toBe(created.id);
|
||||
expect(found?.name).toBe("Beta");
|
||||
expect(missing).toBeNull();
|
||||
});
|
||||
|
||||
it("findAgentByName excludes ephemeral agents", async () => {
|
||||
await store.createAgent({
|
||||
name: "Ephemeral-X",
|
||||
role: "executor",
|
||||
metadata: { agentKind: "task-worker", taskWorker: true },
|
||||
});
|
||||
|
||||
const found = await store.findAgentByName("Ephemeral-X");
|
||||
expect(found).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
it("returns an agent with correct fields", async () => {
|
||||
const agent = await store.createAgent({
|
||||
name: " Test Agent ",
|
||||
|
||||
@@ -472,6 +472,40 @@ export class AgentStore extends EventEmitter {
|
||||
this.db.bumpLastModified();
|
||||
}
|
||||
|
||||
/**
|
||||
* Find the first non-ephemeral agent by exact name.
|
||||
*
|
||||
* Ephemeral task-worker/spawned agents are excluded so callers can use this
|
||||
* for durable identity checks without transient runtime workers conflicting.
|
||||
*
|
||||
* @param name - Agent name to match exactly
|
||||
* @returns Matching non-ephemeral agent, or null when none exists
|
||||
*/
|
||||
async findAgentByName(name: string): Promise<Agent | null> {
|
||||
const rows = this.db
|
||||
.prepare("SELECT * FROM agents WHERE name = ? ORDER BY createdAt DESC")
|
||||
.all(name) as unknown as AgentRow[];
|
||||
|
||||
for (const row of rows) {
|
||||
const agent = this.mapAgentRow(row);
|
||||
if (!isEphemeralAgent(agent)) {
|
||||
return agent;
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
async hasNonEphemeralAgentWithName(name: string): Promise<boolean> {
|
||||
const normalizedName = name.trim();
|
||||
if (!normalizedName) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const existing = await this.findAgentByName(normalizedName);
|
||||
return existing !== null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a new agent with "idle" state.
|
||||
*
|
||||
@@ -484,9 +518,12 @@ export class AgentStore extends EventEmitter {
|
||||
* same default (1h) to both at runtime. Writing the default explicitly
|
||||
* removes that divergence and keeps the persisted config truthful.
|
||||
*
|
||||
* Also enforces non-ephemeral name uniqueness: durable agents cannot share a
|
||||
* name, while ephemeral task-worker agents are allowed to duplicate names.
|
||||
*
|
||||
* @param input - Creation parameters
|
||||
* @returns The created agent
|
||||
* @throws Error if input is invalid
|
||||
* @throws Error if input is invalid or a duplicate non-ephemeral name exists
|
||||
*/
|
||||
async createAgent(input: AgentCreateInput): Promise<Agent> {
|
||||
if (!input.name?.trim()) {
|
||||
@@ -496,10 +533,20 @@ export class AgentStore extends EventEmitter {
|
||||
throw new Error("Agent role is required");
|
||||
}
|
||||
|
||||
const normalizedName = input.name.trim();
|
||||
const metadata = input.metadata ?? {};
|
||||
const ephemeral = isEphemeralAgent({ metadata, name: input.name, role: input.role, reportsTo: input.reportsTo });
|
||||
|
||||
if (!ephemeral) {
|
||||
const existing = await this.findAgentByName(normalizedName);
|
||||
if (existing) {
|
||||
throw new Error(`Agent with name "${normalizedName}" already exists (agentId: ${existing.id})`);
|
||||
}
|
||||
}
|
||||
|
||||
const now = new Date().toISOString();
|
||||
const agentId = `agent-${randomUUID().slice(0, 8)}`;
|
||||
|
||||
const metadata = input.metadata ?? {};
|
||||
const runtimeConfig = resolveCreationRuntimeConfig(input.runtimeConfig, metadata);
|
||||
|
||||
// Default heartbeatProcedurePath for new non-ephemeral agents so operators
|
||||
@@ -508,13 +555,12 @@ export class AgentStore extends EventEmitter {
|
||||
// tweaks to one agent's procedure do not bleed into the rest of the
|
||||
// team. Ephemeral task workers skip this — they're short-lived and
|
||||
// don't need persistent procedure files.
|
||||
const ephemeral = isEphemeralAgent({ metadata, name: input.name, role: input.role, reportsTo: input.reportsTo });
|
||||
const resolvedHeartbeatProcedurePath = input.heartbeatProcedurePath
|
||||
?? (ephemeral ? undefined : getDefaultHeartbeatProcedurePath(agentId, input.name));
|
||||
|
||||
const agent: Agent = {
|
||||
id: agentId,
|
||||
name: input.name.trim(),
|
||||
name: normalizedName,
|
||||
role: input.role,
|
||||
state: "idle",
|
||||
createdAt: now,
|
||||
|
||||
@@ -366,6 +366,33 @@ describe("POST /api/agents/import", () => {
|
||||
expect(mockCreateAgent).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("maps store-level duplicate errors to skipped results", async () => {
|
||||
mockPrepareAgentCompaniesImport.mockReturnValue({
|
||||
items: [{
|
||||
manifestKey: "dup-agent",
|
||||
aliases: ["dup-agent"],
|
||||
index: 0,
|
||||
input: { name: "Dup Agent", role: "custom" },
|
||||
}],
|
||||
result: {
|
||||
created: ["Dup Agent"],
|
||||
skipped: [],
|
||||
errors: [],
|
||||
},
|
||||
});
|
||||
|
||||
mockCreateAgent.mockRejectedValueOnce(new Error("Agent with name \"Dup Agent\" already exists (agentId: agent-existing)"));
|
||||
|
||||
const response = await postImport(app, {
|
||||
manifest: "---\nname: Dup Agent\n---\nInstructions",
|
||||
});
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
const body = response.body as any;
|
||||
expect(body.skipped).toContain("Dup Agent");
|
||||
expect(body.errors).toEqual([]);
|
||||
});
|
||||
|
||||
it("honors skipExisting and returns skipped agents", async () => {
|
||||
mockListAgents.mockResolvedValue([{ id: "agent-existing", name: "YAML Agent" }]);
|
||||
mockPrepareAgentCompaniesImport.mockReturnValue({
|
||||
|
||||
@@ -1840,6 +1840,51 @@ describe("Agent create/update routes", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("POST /api/agents returns 409 for duplicate non-ephemeral names", async () => {
|
||||
const first = await REQUEST(
|
||||
buildAgentApp(),
|
||||
"POST",
|
||||
"/api/agents",
|
||||
JSON.stringify({
|
||||
name: "Duplicate Agent",
|
||||
role: "executor",
|
||||
}),
|
||||
{ "Content-Type": "application/json" },
|
||||
);
|
||||
|
||||
expect(first.status).toBe(201);
|
||||
|
||||
const duplicate = await REQUEST(
|
||||
buildAgentApp(),
|
||||
"POST",
|
||||
"/api/agents",
|
||||
JSON.stringify({
|
||||
name: "Duplicate Agent",
|
||||
role: "reviewer",
|
||||
}),
|
||||
{ "Content-Type": "application/json" },
|
||||
);
|
||||
|
||||
expect(duplicate.status).toBe(409);
|
||||
expect(duplicate.body).toEqual({
|
||||
error: "Agent with this name already exists",
|
||||
name: "Duplicate Agent",
|
||||
});
|
||||
|
||||
const differentName = await REQUEST(
|
||||
buildAgentApp(),
|
||||
"POST",
|
||||
"/api/agents",
|
||||
JSON.stringify({
|
||||
name: "Unique Agent",
|
||||
role: "reviewer",
|
||||
}),
|
||||
{ "Content-Type": "application/json" },
|
||||
);
|
||||
|
||||
expect(differentName.status).toBe(201);
|
||||
});
|
||||
|
||||
it("POST /api/agents returns 400 when soul exceeds 10,000 characters", async () => {
|
||||
const longSoul = "x".repeat(10001);
|
||||
const res = await REQUEST(
|
||||
|
||||
@@ -154,22 +154,32 @@ export function registerAgentCoreListCreateRoutes(ctx: ApiRoutesContext, deps: A
|
||||
const agentStore = new AgentStore({ rootDir: scopedStore.getFusionDir() });
|
||||
await agentStore.init();
|
||||
|
||||
const agent = await agentStore.createAgent({
|
||||
name,
|
||||
role: role as AgentCapability,
|
||||
metadata,
|
||||
title: title ?? undefined,
|
||||
icon: icon ?? undefined,
|
||||
reportsTo: reportsTo ?? undefined,
|
||||
runtimeConfig,
|
||||
permissions,
|
||||
instructionsPath: instructionsPath ?? undefined,
|
||||
instructionsText: instructionsText ?? undefined,
|
||||
soul: soul ?? undefined,
|
||||
memory: memory ?? undefined,
|
||||
bundleConfig: bundleConfig ?? undefined,
|
||||
heartbeatProcedurePath: heartbeatProcedurePath ?? undefined,
|
||||
});
|
||||
let agent: Agent;
|
||||
try {
|
||||
agent = await agentStore.createAgent({
|
||||
name,
|
||||
role: role as AgentCapability,
|
||||
metadata,
|
||||
title: title ?? undefined,
|
||||
icon: icon ?? undefined,
|
||||
reportsTo: reportsTo ?? undefined,
|
||||
runtimeConfig,
|
||||
permissions,
|
||||
instructionsPath: instructionsPath ?? undefined,
|
||||
instructionsText: instructionsText ?? undefined,
|
||||
soul: soul ?? undefined,
|
||||
memory: memory ?? undefined,
|
||||
bundleConfig: bundleConfig ?? undefined,
|
||||
heartbeatProcedurePath: heartbeatProcedurePath ?? undefined,
|
||||
});
|
||||
} catch (err: unknown) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
if (message.includes("Agent with name")) {
|
||||
res.status(409).json({ error: "Agent with this name already exists", name });
|
||||
return;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
|
||||
// Seed the default heartbeat procedure file if the new agent landed on
|
||||
// the per-agent default path (which createAgent fills in for
|
||||
|
||||
@@ -749,10 +749,17 @@ async function persistImportedSkills(
|
||||
created.push({ id: agent.id, name: agent.name });
|
||||
createdAgentIdsByManifestKey.set(item.manifestKey, agent.id);
|
||||
} catch (err: unknown) {
|
||||
if (err instanceof ApiError) {
|
||||
throw err;
|
||||
}
|
||||
errors.push({ name: item.input.name, error: err instanceof Error ? err.message : String(err) });
|
||||
if (err instanceof ApiError) {
|
||||
throw err;
|
||||
}
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
if (message.includes("Agent with name")) {
|
||||
if (!result.skipped.includes(item.input.name)) {
|
||||
result.skipped.push(item.input.name);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
errors.push({ name: item.input.name, error: message });
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
11
packages/engine/src/__tests__/in-process-runtime.test.ts
Normal file
11
packages/engine/src/__tests__/in-process-runtime.test.ts
Normal file
@@ -0,0 +1,11 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
|
||||
describe("InProcessRuntime onStart duplicate guard", () => {
|
||||
it("contains a taskAgentMap guard before creating task-worker agents", () => {
|
||||
const source = readFileSync(join(process.cwd(), "src/runtimes/in-process-runtime.ts"), "utf-8");
|
||||
expect(source).toContain("if (this.taskAgentMap.has(task.id))");
|
||||
expect(source).toContain("Skipping task-worker creation for");
|
||||
});
|
||||
});
|
||||
@@ -627,6 +627,24 @@ describe("InProcessRuntime", () => {
|
||||
expect(assignTaskSpy.mock.invocationCallOrder[0]).toBeLessThan(updateStateSpy.mock.invocationCallOrder[0]);
|
||||
}, 30000);
|
||||
|
||||
it("does not create duplicate task-worker agents when onStart fires twice for one task", async () => {
|
||||
await runtime.start();
|
||||
|
||||
const store = getAgentStore(runtime);
|
||||
const executorOptions = mockExecutorCtor.mock.calls.at(-1)?.[0] as {
|
||||
onStart?: (task: Task, worktreePath: string) => void;
|
||||
};
|
||||
|
||||
executorOptions.onStart?.({ id: "FN-DUP-ONSTART" } as Task, join(testDir, "worktree-FN-DUP-ONSTART"));
|
||||
executorOptions.onStart?.({ id: "FN-DUP-ONSTART" } as Task, join(testDir, "worktree-FN-DUP-ONSTART"));
|
||||
|
||||
await vi.waitFor(async () => {
|
||||
const agents = await store.listAgents({ includeEphemeral: true });
|
||||
const matching = agents.filter((agent: Agent) => agent.name === "executor-FN-DUP-ONSTART");
|
||||
expect(matching).toHaveLength(1);
|
||||
});
|
||||
}, 30000);
|
||||
|
||||
it("does not wake executeHeartbeat for runtime task-worker assignment events", async () => {
|
||||
await runtime.start();
|
||||
|
||||
|
||||
@@ -377,6 +377,12 @@ export class InProcessRuntime
|
||||
// These workers are not heartbeat-managed dashboard agents, so mark them
|
||||
// explicitly and disable heartbeat triggers/timers.
|
||||
if (this.agentStore) {
|
||||
if (this.taskAgentMap.has(task.id)) {
|
||||
runtimeLog.warn(`Skipping task-worker creation for ${task.id}: agent already exists (${this.taskAgentMap.get(task.id)})`);
|
||||
return;
|
||||
}
|
||||
|
||||
this.taskAgentMap.set(task.id, "creating");
|
||||
this.agentStore.createAgent({
|
||||
name: `executor-${task.id}`,
|
||||
role: "executor",
|
||||
@@ -394,6 +400,7 @@ export class InProcessRuntime
|
||||
await this.agentStore!.updateAgentState(agent.id, "active");
|
||||
await this.agentStore!.updateAgentState(agent.id, "running");
|
||||
}).catch((err: unknown) => {
|
||||
this.taskAgentMap.delete(task.id);
|
||||
runtimeLog.warn(`Failed to create agent for task ${task.id}:`, err);
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user