fix(cli): validate agentId in fn_task_create/update and unblock bundle tests
fn_task_create and fn_task_update accepted any string as `agentId` and wrote it verbatim onto `task.assignedAgentId`, letting hallucinated IDs (e.g. `agent-executor-001`) appear as agent badges in the dashboard. Mirror the validation already used by fn_delegate: look the agent up via AgentStore and reject unknown or ephemeral/runtime-managed agents. Null still clears the field on update. Also clean up two stale failures in bundle-output.test that predated this change: - pi-claude-cli no longer imports cross-spawn, so drop the dependency and its orphan type-decl file. - Loosen the spawn-import regex to match `spawn` anywhere in the destructured import (the source has additional named imports). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -119,7 +119,7 @@ describe("CLI bundle output", () => {
|
||||
it("pi-claude-cli source imports spawn from node:child_process", () => {
|
||||
const processManagerSource = readFileSync(join(cliRoot, "dist", "pi-claude-cli", "src", "process-manager.ts"), "utf-8");
|
||||
|
||||
expect(processManagerSource).toMatch(/import\s+\{\s*spawn[\s\S]*\}\s+from\s*["']node:child_process["']/);
|
||||
expect(processManagerSource).toMatch(/import\s+\{[^}]*\bspawn\b[^}]*\}\s+from\s*["']node:child_process["']/);
|
||||
});
|
||||
|
||||
it("pi-claude-cli package.json does not require cross-spawn dependency", () => {
|
||||
|
||||
@@ -27,7 +27,7 @@ vi.mock("../commands/task.js", () => ({
|
||||
}));
|
||||
|
||||
import kbExtension from "../extension.js";
|
||||
import { TaskStore } from "@fusion/core";
|
||||
import { TaskStore, AgentStore } from "@fusion/core";
|
||||
import { isGhAvailable, isGhAuthenticated, runGhJsonAsync } from "@fusion/core/gh-cli";
|
||||
import { runTaskPlan } from "../commands/task.js";
|
||||
|
||||
@@ -80,6 +80,20 @@ function makeCtx(cwd: string) {
|
||||
return { cwd } as any;
|
||||
}
|
||||
|
||||
async function seedAgent(
|
||||
cwd: string,
|
||||
overrides: { ephemeral?: boolean; name?: string } = {},
|
||||
): Promise<string> {
|
||||
const agentStore = new AgentStore({ rootDir: join(cwd, ".fusion") });
|
||||
await agentStore.init();
|
||||
const agent = await agentStore.createAgent({
|
||||
name: overrides.name ?? "test-agent",
|
||||
role: "executor",
|
||||
metadata: overrides.ephemeral ? { agentKind: "task-worker" } : {},
|
||||
});
|
||||
return agent.id;
|
||||
}
|
||||
|
||||
async function removeDirWithRetries(path: string) {
|
||||
const maxAttempts = 4;
|
||||
|
||||
@@ -250,23 +264,53 @@ describe.skip("fn pi extension", () => {
|
||||
});
|
||||
|
||||
it("creates a task with assigned agent ID", async () => {
|
||||
const agentId = await seedAgent(tmpDir);
|
||||
const tool = api.tools.get("fn_task_create")!;
|
||||
const result = await tool.execute(
|
||||
"call-1",
|
||||
{ description: "Task with assignee", agentId: "agent-abc123" },
|
||||
{ description: "Task with assignee", agentId },
|
||||
undefined,
|
||||
undefined,
|
||||
makeCtx(tmpDir),
|
||||
);
|
||||
|
||||
expect(result.details.taskId).toBe("FN-001");
|
||||
expect(result.details.assignedAgentId).toBe("agent-abc123");
|
||||
expect(result.content[0].text).toContain("Assigned to: agent-abc123");
|
||||
expect(result.details.assignedAgentId).toBe(agentId);
|
||||
expect(result.content[0].text).toContain(`Assigned to: ${agentId}`);
|
||||
|
||||
// Verify persistence via show
|
||||
const showTool = api.tools.get("fn_task_show")!;
|
||||
const show = await showTool.execute("s1", { id: "FN-001" }, undefined, undefined, makeCtx(tmpDir));
|
||||
expect(show.details.task.assignedAgentId).toBe("agent-abc123");
|
||||
expect(show.details.task.assignedAgentId).toBe(agentId);
|
||||
});
|
||||
|
||||
it("rejects unknown agent IDs", async () => {
|
||||
const tool = api.tools.get("fn_task_create")!;
|
||||
const result = await tool.execute(
|
||||
"call-1",
|
||||
{ description: "Task with bogus assignee", agentId: "agent-does-not-exist" },
|
||||
undefined,
|
||||
undefined,
|
||||
makeCtx(tmpDir),
|
||||
);
|
||||
|
||||
expect(result.isError).toBe(true);
|
||||
expect(result.content[0].text).toContain("Agent agent-does-not-exist not found");
|
||||
});
|
||||
|
||||
it("rejects ephemeral/runtime-managed agents", async () => {
|
||||
const ephemeralId = await seedAgent(tmpDir, { ephemeral: true, name: "task-worker" });
|
||||
const tool = api.tools.get("fn_task_create")!;
|
||||
const result = await tool.execute(
|
||||
"call-1",
|
||||
{ description: "Task with worker assignee", agentId: ephemeralId },
|
||||
undefined,
|
||||
undefined,
|
||||
makeCtx(tmpDir),
|
||||
);
|
||||
|
||||
expect(result.isError).toBe(true);
|
||||
expect(result.content[0].text).toContain("ephemeral/runtime agent");
|
||||
});
|
||||
|
||||
it("creates a task without assigned agent ID by default", async () => {
|
||||
@@ -364,10 +408,11 @@ describe.skip("fn pi extension", () => {
|
||||
const createTool = api.tools.get("fn_task_create")!;
|
||||
await createTool.execute("c1", { description: "Original" }, undefined, undefined, makeCtx(tmpDir));
|
||||
|
||||
const agentId = await seedAgent(tmpDir);
|
||||
const updateTool = api.tools.get("fn_task_update")!;
|
||||
const result = await updateTool.execute(
|
||||
"u1",
|
||||
{ id: "FN-001", agentId: "agent-abc123" },
|
||||
{ id: "FN-001", agentId },
|
||||
undefined,
|
||||
undefined,
|
||||
makeCtx(tmpDir),
|
||||
@@ -379,14 +424,32 @@ describe.skip("fn pi extension", () => {
|
||||
|
||||
const showTool = api.tools.get("fn_task_show")!;
|
||||
const show = await showTool.execute("s1", { id: "FN-001" }, undefined, undefined, makeCtx(tmpDir));
|
||||
expect(show.details.task.assignedAgentId).toBe("agent-abc123");
|
||||
expect(show.details.task.assignedAgentId).toBe(agentId);
|
||||
});
|
||||
|
||||
it("rejects unknown agent IDs on update", async () => {
|
||||
const createTool = api.tools.get("fn_task_create")!;
|
||||
await createTool.execute("c1", { description: "Original" }, undefined, undefined, makeCtx(tmpDir));
|
||||
|
||||
const updateTool = api.tools.get("fn_task_update")!;
|
||||
const result = await updateTool.execute(
|
||||
"u1",
|
||||
{ id: "FN-001", agentId: "agent-does-not-exist" },
|
||||
undefined,
|
||||
undefined,
|
||||
makeCtx(tmpDir),
|
||||
);
|
||||
|
||||
expect(result.isError).toBe(true);
|
||||
expect(result.content[0].text).toContain("Agent agent-does-not-exist not found");
|
||||
});
|
||||
|
||||
it("clears task assigned agent ID with null", async () => {
|
||||
const agentId = await seedAgent(tmpDir);
|
||||
const createTool = api.tools.get("fn_task_create")!;
|
||||
await createTool.execute(
|
||||
"c1",
|
||||
{ description: "Original", agentId: "agent-abc123" },
|
||||
{ description: "Original", agentId },
|
||||
undefined,
|
||||
undefined,
|
||||
makeCtx(tmpDir),
|
||||
|
||||
@@ -70,6 +70,30 @@ function getFusionDir(cwd: string): string {
|
||||
return join(resolveProjectRoot(cwd), ".fusion");
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate an agent id supplied to task create/update tools.
|
||||
* Returns null on success, or an error message describing why the id was rejected.
|
||||
*
|
||||
* Rejects unknown agents and ephemeral/runtime-managed agents — mirrors fn_delegate
|
||||
* so callers can't park hallucinated or task-worker IDs in `task.assignedAgentId`.
|
||||
*/
|
||||
async function validateAssignableAgentId(
|
||||
cwd: string,
|
||||
agentId: string,
|
||||
): Promise<string | null> {
|
||||
const { AgentStore, isEphemeralAgent } = await import("@fusion/core");
|
||||
const agentStore = new AgentStore({ rootDir: getFusionDir(cwd) });
|
||||
await agentStore.init();
|
||||
const agent = await agentStore.getAgent(agentId);
|
||||
if (!agent) {
|
||||
return `Agent ${agentId} not found`;
|
||||
}
|
||||
if (isEphemeralAgent(agent)) {
|
||||
return `Cannot assign task to ephemeral/runtime agent ${agentId}`;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function formatTaskLine(t: Task): string {
|
||||
const label =
|
||||
t.title || t.description.slice(0, 60) + (t.description.length > 60 ? "…" : "");
|
||||
@@ -167,6 +191,18 @@ export default function kbExtension(pi: ExtensionAPI) {
|
||||
|
||||
async execute(_toolCallId, params, _signal, _onUpdate, ctx) {
|
||||
const store = await getStore(ctx.cwd);
|
||||
|
||||
if (params.agentId !== undefined) {
|
||||
const error = await validateAssignableAgentId(ctx.cwd, params.agentId);
|
||||
if (error) {
|
||||
return {
|
||||
content: [{ type: "text", text: error }],
|
||||
isError: true,
|
||||
details: { error },
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
const task = await store.createTask({
|
||||
description: params.description.trim(),
|
||||
dependencies: params.depends,
|
||||
@@ -273,6 +309,16 @@ export default function kbExtension(pi: ExtensionAPI) {
|
||||
updatedFields.push("dependencies");
|
||||
}
|
||||
if (params.agentId !== undefined) {
|
||||
if (params.agentId !== null) {
|
||||
const error = await validateAssignableAgentId(ctx.cwd, params.agentId);
|
||||
if (error) {
|
||||
return {
|
||||
content: [{ type: "text", text: error }],
|
||||
isError: true,
|
||||
details: { error },
|
||||
};
|
||||
}
|
||||
}
|
||||
updates.assignedAgentId = params.agentId;
|
||||
updatedFields.push("agentId");
|
||||
}
|
||||
|
||||
@@ -23,9 +23,6 @@
|
||||
"@mariozechner/pi-ai": "*",
|
||||
"@mariozechner/pi-coding-agent": "*"
|
||||
},
|
||||
"dependencies": {
|
||||
"cross-spawn": "^7.0.6"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@types/node": "^22.0.0",
|
||||
"typescript": "^5.7.0",
|
||||
|
||||
@@ -1,7 +0,0 @@
|
||||
declare module "cross-spawn" {
|
||||
const spawn: typeof import("node:child_process").spawn & {
|
||||
sync: typeof import("node:child_process").spawnSync;
|
||||
};
|
||||
|
||||
export default spawn;
|
||||
}
|
||||
Reference in New Issue
Block a user