feat(FN-3935): align engineer role routing with explicit assignment and del
Centralized routing policy helpers in core and aligned both direct assignment and delegation to route engineer-role agents consistently, ensuring assigned tasks respect explicit engineer routing the same way delegated tasks do. Added comprehensive test coverage across core, CLI, dashboard, and engin Fusion-Task-Id: FN-3935 Fusion-Task-Lineage: 069f4d54-f7a8-4bd3-a2c3-4de830de042f
This commit is contained in:
@@ -1,7 +1,10 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
canAgentTakeImplementationTask,
|
||||
canAgentTakeImplementationTaskForBacklogPickup,
|
||||
canAgentTakeImplementationTaskForExplicitRouting,
|
||||
formatRoleMismatchReason,
|
||||
isEngineerRoleAgent,
|
||||
isExecutorRoleAgent,
|
||||
isImplementationTask,
|
||||
} from "../agent-role-policy.js";
|
||||
@@ -19,17 +22,36 @@ describe("agent-role-policy", () => {
|
||||
expect(isImplementationTask({ column: "archived" })).toBe(false);
|
||||
});
|
||||
|
||||
it("allows executor agents to take implementation tasks", () => {
|
||||
it("allows executor agents in both explicit routing and backlog pickup", () => {
|
||||
expect(isExecutorRoleAgent({ role: "executor" })).toBe(true);
|
||||
expect(
|
||||
canAgentTakeImplementationTaskForExplicitRouting({ role: "executor" }, { column: "todo" }),
|
||||
).toBe(true);
|
||||
expect(
|
||||
canAgentTakeImplementationTaskForBacklogPickup({ role: "executor" }, { column: "todo" }),
|
||||
).toBe(true);
|
||||
expect(
|
||||
canAgentTakeImplementationTask({ role: "executor" }, { column: "todo" }),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects non-executor agents for implementation tasks", () => {
|
||||
it("allows durable engineer only for explicit routing", () => {
|
||||
expect(isEngineerRoleAgent({ role: "engineer" })).toBe(true);
|
||||
expect(
|
||||
canAgentTakeImplementationTaskForExplicitRouting({ role: "engineer" }, { column: "todo" }),
|
||||
).toBe(true);
|
||||
expect(
|
||||
canAgentTakeImplementationTaskForBacklogPickup({ role: "engineer" }, { column: "todo" }),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("keeps reviewer blocked by default", () => {
|
||||
expect(isExecutorRoleAgent({ role: "reviewer" })).toBe(false);
|
||||
expect(
|
||||
canAgentTakeImplementationTask({ role: "reviewer" }, { column: "todo" }),
|
||||
canAgentTakeImplementationTaskForExplicitRouting({ role: "reviewer" }, { column: "todo" }),
|
||||
).toBe(false);
|
||||
expect(
|
||||
canAgentTakeImplementationTaskForBacklogPickup({ role: "reviewer" }, { column: "todo" }),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
@@ -41,5 +63,7 @@ describe("agent-role-policy", () => {
|
||||
expect(reason).toContain("agent-1");
|
||||
expect(reason).toContain("reviewer");
|
||||
expect(reason).toContain("FN-123");
|
||||
expect(reason).toContain("requires an \"executor\"-role agent by default");
|
||||
expect(reason).toContain("durable \"engineer\" supported only for explicit routing");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1975,6 +1975,32 @@ describe("AgentStore", () => {
|
||||
expect(result.ok).toBe(false);
|
||||
if (result.ok) return;
|
||||
expect(result.reason).toMatch(/requires an "executor"-role agent/);
|
||||
expect(result.reason).toMatch(/durable "engineer" supported only for explicit routing/);
|
||||
|
||||
const claimedTask = await taskStore.getTask(taskId);
|
||||
expect(claimedTask?.assignedAgentId).toBeUndefined();
|
||||
});
|
||||
|
||||
it("claimTaskForAgent allows engineer claim for explicitly assigned implementation tasks", async () => {
|
||||
const engineer = await store.createAgent({ name: "Engineer", role: "engineer" });
|
||||
await taskStore.updateTask(taskId, { assignedAgentId: engineer.id });
|
||||
|
||||
const result = await store.claimTaskForAgent(engineer.id, taskId);
|
||||
expect(result.ok).toBe(true);
|
||||
if (!result.ok) return;
|
||||
|
||||
const claimedTask = await taskStore.getTask(taskId);
|
||||
expect(claimedTask?.assignedAgentId).toBe(engineer.id);
|
||||
expect(claimedTask?.checkedOutBy).toBe(engineer.id);
|
||||
});
|
||||
|
||||
it("claimTaskForAgent rejects engineer auto-claim for unassigned implementation tasks", async () => {
|
||||
const engineer = await store.createAgent({ name: "Engineer", role: "engineer" });
|
||||
|
||||
const result = await store.claimTaskForAgent(engineer.id, taskId);
|
||||
expect(result.ok).toBe(false);
|
||||
if (result.ok) return;
|
||||
expect(result.reason).toMatch(/requires an "executor"-role agent/);
|
||||
|
||||
const claimedTask = await taskStore.getTask(taskId);
|
||||
expect(claimedTask?.assignedAgentId).toBeUndefined();
|
||||
|
||||
@@ -296,6 +296,33 @@ describe("TaskStore", () => {
|
||||
expect(selected?.priority).toBe("todo");
|
||||
});
|
||||
|
||||
it("returns assigned implementation todos for engineer role agents", async () => {
|
||||
const todo = await store.createTask({
|
||||
description: "Assigned engineer todo",
|
||||
column: "todo",
|
||||
assignedAgentId: "agent-1",
|
||||
});
|
||||
|
||||
const selected = await store.selectNextTaskForAgent("agent-1", {
|
||||
id: "agent-1",
|
||||
role: "engineer",
|
||||
});
|
||||
|
||||
expect(selected?.task.id).toBe(todo.id);
|
||||
expect(selected?.priority).toBe("todo");
|
||||
});
|
||||
|
||||
it("does not auto-claim unassigned implementation backlog for engineer role agents", async () => {
|
||||
await store.createTask({
|
||||
description: "Unassigned todo",
|
||||
column: "todo",
|
||||
});
|
||||
|
||||
await expect(
|
||||
store.selectNextTaskForAgent("agent-1", { id: "agent-1", role: "engineer" }),
|
||||
).resolves.toBeNull();
|
||||
});
|
||||
|
||||
it("allows non-executor role agents to pick assigned todos when override metadata is set", async () => {
|
||||
const delegated = await store.createTask({
|
||||
description: "Assigned todo override",
|
||||
|
||||
@@ -15,16 +15,34 @@ export function isExecutorRoleAgent(agent: Pick<Agent, "role">): boolean {
|
||||
return agent.role === "executor";
|
||||
}
|
||||
|
||||
export function canAgentTakeImplementationTask(
|
||||
export function isEngineerRoleAgent(agent: Pick<Agent, "role">): boolean {
|
||||
return agent.role === "engineer";
|
||||
}
|
||||
|
||||
export function canAgentTakeImplementationTaskForExplicitRouting(
|
||||
agent: Pick<Agent, "role">,
|
||||
task: Pick<Task, "column">,
|
||||
): boolean {
|
||||
return !isImplementationTask(task) || isExecutorRoleAgent(agent) || isEngineerRoleAgent(agent);
|
||||
}
|
||||
|
||||
export function canAgentTakeImplementationTaskForBacklogPickup(
|
||||
agent: Pick<Agent, "role">,
|
||||
task: Pick<Task, "column">,
|
||||
): boolean {
|
||||
return !isImplementationTask(task) || isExecutorRoleAgent(agent);
|
||||
}
|
||||
|
||||
export function canAgentTakeImplementationTask(
|
||||
agent: Pick<Agent, "role">,
|
||||
task: Pick<Task, "column">,
|
||||
): boolean {
|
||||
return canAgentTakeImplementationTaskForBacklogPickup(agent, task);
|
||||
}
|
||||
|
||||
export function formatRoleMismatchReason(
|
||||
agent: Pick<Agent, "id" | "role">,
|
||||
task: Pick<Task, "id" | "column">,
|
||||
): string {
|
||||
return `Agent ${agent.id} has role "${agent.role}"; implementation task ${task.id} requires an "executor"-role agent. Pass override=true to bypass.`;
|
||||
return `Agent ${agent.id} has role "${agent.role}"; implementation task ${task.id} requires an "executor"-role agent by default, with durable "engineer" supported only for explicit routing. Pass override=true to bypass.`;
|
||||
}
|
||||
|
||||
@@ -62,7 +62,7 @@ interface CheckoutLeaseContext {
|
||||
renewedAt?: string;
|
||||
}
|
||||
import { computeAccessState } from "./agent-permissions.js";
|
||||
import { canAgentTakeImplementationTask, formatRoleMismatchReason } from "./agent-role-policy.js";
|
||||
import { canAgentTakeImplementationTask, canAgentTakeImplementationTaskForExplicitRouting, formatRoleMismatchReason } from "./agent-role-policy.js";
|
||||
import { resolveEffectiveAgentPermissionPolicy } from "./agent-permission-policy.js";
|
||||
import { Database } from "./db.js";
|
||||
import { createAgentRunSnapshot, createAgentSnapshot, validateSnapshotEnvelope, type AgentRunSnapshot, type AgentSnapshot } from "./shared-mesh-state.js";
|
||||
@@ -1331,7 +1331,11 @@ export class AgentStore extends EventEmitter {
|
||||
return { ok: false, reason: "paused", task };
|
||||
}
|
||||
|
||||
if (!canAgentTakeImplementationTask(agent, task)) {
|
||||
const isExplicitlyAssignedToAgent = task.assignedAgentId === agentId;
|
||||
const roleAllowed = isExplicitlyAssignedToAgent
|
||||
? canAgentTakeImplementationTaskForExplicitRouting(agent, task)
|
||||
: canAgentTakeImplementationTask(agent, task);
|
||||
if (!roleAllowed) {
|
||||
return { ok: false, reason: formatRoleMismatchReason(agent, task), task };
|
||||
}
|
||||
|
||||
|
||||
@@ -68,6 +68,8 @@ export {
|
||||
isImplementationTask,
|
||||
isExecutorRoleAgent,
|
||||
canAgentTakeImplementationTask,
|
||||
canAgentTakeImplementationTaskForExplicitRouting,
|
||||
canAgentTakeImplementationTaskForBacklogPickup,
|
||||
formatRoleMismatchReason,
|
||||
} from "./agent-role-policy.js";
|
||||
export { ReflectionStore } from "./reflection-store.js";
|
||||
|
||||
@@ -7,7 +7,7 @@ import type { Task, TaskDetail, TaskCreateInput, TaskAttachment, AgentLogEntry,
|
||||
import { createActivityLogSnapshot, createRunAuditSnapshot, createTaskMetadataSnapshot, toTaskMetadataRecord, validateSnapshotEnvelope, type ActivityLogSnapshot, type RunAuditSnapshot, type TaskMetadataSnapshot } from "./shared-mesh-state.js";
|
||||
import { VALID_TRANSITIONS, DEFAULT_SETTINGS, isGlobalOnlySettingsKey, WORKFLOW_STEP_TEMPLATES, validateDocumentKey } from "./types.js";
|
||||
import { normalizeTaskPriority } from "./task-priority.js";
|
||||
import { canAgentTakeImplementationTask } from "./agent-role-policy.js";
|
||||
import { canAgentTakeImplementationTaskForExplicitRouting } from "./agent-role-policy.js";
|
||||
import { GlobalSettingsStore } from "./global-settings.js";
|
||||
import { Database, toJson, toJsonNullable, fromJson } from "./db.js";
|
||||
import { ArchiveDatabase } from "./archive-db.js";
|
||||
@@ -3117,7 +3117,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
if (task.column === "in-progress" || hasExecutorRoleOverride(task)) {
|
||||
return true;
|
||||
}
|
||||
return canAgentTakeImplementationTask(agent, task);
|
||||
return canAgentTakeImplementationTaskForExplicitRouting(agent, task);
|
||||
})
|
||||
: assignedTasks;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user