feat(FN-3507): block dispatch when project lacks node mapping
Adds validation to block task dispatch when no project-node mapping exists (FN-3507), including a read helper in CentralCore and a new `node-dispatch-validation` module integrated into the scheduler and in-process runtime, with test coverage across routing and validation scenarios. Fusion-Task-Id: FN-3507
This commit is contained in:
@@ -804,6 +804,29 @@ describe("CentralCore", () => {
|
||||
await expect(central.getProjectNodePathMapping(project.id, nodeA.id)).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it("should return exact mapped path via getProjectNodePath and undefined for unmapped pairs", async () => {
|
||||
const projectPath = join(tempDir, "mapping-read-project");
|
||||
mkdirSync(projectPath);
|
||||
projectPaths.push(projectPath);
|
||||
|
||||
const project = await central.registerProject({
|
||||
name: "Mapping Read Project",
|
||||
path: projectPath,
|
||||
});
|
||||
const mappedNode = await central.registerNode({ name: "mapping-read-node", type: "local" });
|
||||
const otherNode = await central.registerNode({ name: "mapping-read-node-other", type: "local" });
|
||||
|
||||
await central.upsertProjectNodePathMapping({
|
||||
projectId: project.id,
|
||||
nodeId: mappedNode.id,
|
||||
path: "/mapped/node/path",
|
||||
});
|
||||
|
||||
await expect(central.getProjectNodePath(project.id, mappedNode.id)).resolves.toBe("/mapped/node/path");
|
||||
await expect(central.getProjectNodePath(project.id, otherNode.id)).resolves.toBeUndefined();
|
||||
await expect(central.getProjectNodePath("proj_missing", mappedNode.id)).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it("should validate project and node existence for mapping APIs", async () => {
|
||||
const node = await central.registerNode({ name: "mapping-validation-node", type: "local" });
|
||||
|
||||
|
||||
@@ -1769,6 +1769,16 @@ export class CentralCore extends EventEmitter<CentralCoreEvents> {
|
||||
return row ? this.rowToProjectNodePathMapping(row) : undefined;
|
||||
}
|
||||
|
||||
async getProjectNodePath(projectId: string, nodeId: string): Promise<string | undefined> {
|
||||
this.ensureInitialized();
|
||||
|
||||
const row = this.db!
|
||||
.prepare("SELECT path FROM projectNodePathMappings WHERE projectId = ? AND nodeId = ?")
|
||||
.get(projectId, nodeId) as { path: string } | undefined;
|
||||
|
||||
return row?.path;
|
||||
}
|
||||
|
||||
async listProjectNodePathMappings(filters?: {
|
||||
projectId?: string;
|
||||
nodeId?: string;
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { validateProjectNodeMapping } from "../node-dispatch-validation.js";
|
||||
|
||||
describe("validateProjectNodeMapping", () => {
|
||||
it("allows dispatch when mapping path is present", () => {
|
||||
expect(
|
||||
validateProjectNodeMapping({ nodeId: "node-1", mappedPath: "/work/project" }),
|
||||
).toEqual({ allowed: true });
|
||||
});
|
||||
|
||||
it("blocks dispatch when mapping is missing or blank", () => {
|
||||
expect(
|
||||
validateProjectNodeMapping({ nodeId: "node-1", mappedPath: undefined }),
|
||||
).toEqual({
|
||||
allowed: false,
|
||||
code: "missing-project-mapping",
|
||||
reason: "Execution blocked: project has no path mapping for node node-1",
|
||||
});
|
||||
|
||||
expect(
|
||||
validateProjectNodeMapping({ nodeId: "node-1", mappedPath: " " }),
|
||||
).toEqual({
|
||||
allowed: false,
|
||||
code: "missing-project-mapping",
|
||||
reason: "Execution blocked: project has no path mapping for node node-1",
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -67,6 +67,10 @@ function createMockHealthMonitor(statusMap: Record<string, NodeStatus | undefine
|
||||
} as unknown as import("../node-health-monitor.js").NodeHealthMonitor;
|
||||
}
|
||||
|
||||
function allowDispatchValidator() {
|
||||
return vi.fn().mockResolvedValue({ allowed: true } as const);
|
||||
}
|
||||
|
||||
describe("Scheduler node routing", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
@@ -134,6 +138,139 @@ describe("Scheduler node routing", () => {
|
||||
expect(scheduler).toBeDefined();
|
||||
});
|
||||
|
||||
it("dispatches when node mapping validator allows execution", async () => {
|
||||
const task = createMockTask({ id: "FN-112", nodeId: "node-mapped" });
|
||||
const store = createMockStore(task, { maxConcurrent: 1, maxWorktrees: 1 });
|
||||
const validateNodeDispatch = allowDispatchValidator();
|
||||
const scheduler = new Scheduler(store, { validateNodeDispatch });
|
||||
(scheduler as unknown as { running: boolean }).running = true;
|
||||
|
||||
await scheduler.schedule();
|
||||
|
||||
expect(validateNodeDispatch).toHaveBeenCalledWith("node-mapped");
|
||||
expect(store.updateTask).toHaveBeenCalledWith(task.id, expect.objectContaining({
|
||||
effectiveNodeId: "node-mapped",
|
||||
effectiveNodeSource: "task-override",
|
||||
}));
|
||||
});
|
||||
|
||||
it("blocks dispatch when node mapping validator fails", async () => {
|
||||
const task = createMockTask({ id: "FN-113", nodeId: "node-unmapped" });
|
||||
const store = createMockStore(task, { maxConcurrent: 1, maxWorktrees: 1 });
|
||||
const validateNodeDispatch = vi.fn().mockResolvedValue({
|
||||
allowed: false,
|
||||
code: "missing-project-mapping",
|
||||
reason: "Execution blocked: project has no path mapping for node node-unmapped",
|
||||
} as const);
|
||||
const scheduler = new Scheduler(store, { validateNodeDispatch });
|
||||
(scheduler as unknown as { running: boolean }).running = true;
|
||||
|
||||
await scheduler.schedule();
|
||||
|
||||
expect(store.updateTask).not.toHaveBeenCalled();
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
task.id,
|
||||
"Execution blocked: project has no path mapping for node node-unmapped",
|
||||
);
|
||||
});
|
||||
|
||||
it("deduplicates missing-mapping block logs across schedule cycles", async () => {
|
||||
const task = createMockTask({ id: "FN-114", nodeId: "node-unmapped" });
|
||||
const store = createMockStore(task, { maxConcurrent: 1, maxWorktrees: 1 });
|
||||
const validateNodeDispatch = vi.fn().mockResolvedValue({
|
||||
allowed: false,
|
||||
code: "missing-project-mapping",
|
||||
reason: "Execution blocked: project has no path mapping for node node-unmapped",
|
||||
} as const);
|
||||
const scheduler = new Scheduler(store, { validateNodeDispatch });
|
||||
(scheduler as unknown as { running: boolean }).running = true;
|
||||
|
||||
await scheduler.schedule();
|
||||
await scheduler.schedule();
|
||||
|
||||
const missingMappingLogs = vi.mocked(store.logEntry).mock.calls.filter(([, message]) =>
|
||||
String(message).includes("project has no path mapping for node node-unmapped"),
|
||||
);
|
||||
expect(missingMappingLogs).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("clears missing-mapping dedup state after successful dispatch", async () => {
|
||||
const task = createMockTask({ id: "FN-115", nodeId: "node-flappy" });
|
||||
const store = createMockStore(task, { maxConcurrent: 1, maxWorktrees: 1 });
|
||||
const validateNodeDispatch = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce({
|
||||
allowed: false,
|
||||
code: "missing-project-mapping",
|
||||
reason: "Execution blocked: project has no path mapping for node node-flappy",
|
||||
} as const)
|
||||
.mockResolvedValueOnce({ allowed: true } as const)
|
||||
.mockResolvedValueOnce({
|
||||
allowed: false,
|
||||
code: "missing-project-mapping",
|
||||
reason: "Execution blocked: project has no path mapping for node node-flappy",
|
||||
} as const);
|
||||
const scheduler = new Scheduler(store, { validateNodeDispatch });
|
||||
(scheduler as unknown as { running: boolean }).running = true;
|
||||
|
||||
await scheduler.schedule();
|
||||
await scheduler.schedule();
|
||||
await scheduler.schedule();
|
||||
|
||||
const missingMappingLogs = vi.mocked(store.logEntry).mock.calls.filter(([, message]) =>
|
||||
String(message).includes("project has no path mapping for node node-flappy"),
|
||||
);
|
||||
expect(missingMappingLogs).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("does not run unavailable-node fallback policy when mapping validation fails", async () => {
|
||||
const task = createMockTask({ id: "FN-116", nodeId: "node-error" });
|
||||
const store = createMockStore(task, {
|
||||
maxConcurrent: 1,
|
||||
maxWorktrees: 1,
|
||||
unavailableNodePolicy: "fallback-local",
|
||||
});
|
||||
const healthMonitor = createMockHealthMonitor({ "node-error": "error" });
|
||||
const validateNodeDispatch = vi.fn().mockResolvedValue({
|
||||
allowed: false,
|
||||
code: "missing-project-mapping",
|
||||
reason: "Execution blocked: project has no path mapping for node node-error",
|
||||
} as const);
|
||||
const scheduler = new Scheduler(store, { nodeHealthMonitor: healthMonitor, validateNodeDispatch });
|
||||
(scheduler as unknown as { running: boolean }).running = true;
|
||||
|
||||
await scheduler.schedule();
|
||||
|
||||
expect((healthMonitor.getNodeHealth as ReturnType<typeof vi.fn>)).not.toHaveBeenCalled();
|
||||
expect(store.updateTask).not.toHaveBeenCalled();
|
||||
expect(store.logEntry).not.toHaveBeenCalledWith(
|
||||
task.id,
|
||||
"Node node-error is error; falling back to local per policy",
|
||||
);
|
||||
});
|
||||
|
||||
it("preserves health-based fallback behavior for mapped nodes", async () => {
|
||||
const task = createMockTask({ id: "FN-117", nodeId: "node-error" });
|
||||
const store = createMockStore(task, {
|
||||
maxConcurrent: 1,
|
||||
maxWorktrees: 1,
|
||||
unavailableNodePolicy: "fallback-local",
|
||||
});
|
||||
const healthMonitor = createMockHealthMonitor({ "node-error": "error" });
|
||||
const validateNodeDispatch = allowDispatchValidator();
|
||||
const scheduler = new Scheduler(store, { nodeHealthMonitor: healthMonitor, validateNodeDispatch });
|
||||
(scheduler as unknown as { running: boolean }).running = true;
|
||||
|
||||
await scheduler.schedule();
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith(task.id, expect.objectContaining({
|
||||
effectiveNodeId: null,
|
||||
effectiveNodeSource: "local",
|
||||
}));
|
||||
expect(store.logEntry).toHaveBeenCalledWith(task.id, "Node node-error is error; falling back to local per policy");
|
||||
});
|
||||
|
||||
it("blocks dispatch when node is unhealthy and policy is block", async () => {
|
||||
const task = createMockTask({ id: "FN-104", nodeId: "node-offline" });
|
||||
const store = createMockStore(task, { maxConcurrent: 1, maxWorktrees: 1, unavailableNodePolicy: "block" });
|
||||
|
||||
19
packages/engine/src/node-dispatch-validation.ts
Normal file
19
packages/engine/src/node-dispatch-validation.ts
Normal file
@@ -0,0 +1,19 @@
|
||||
export type NodeDispatchValidationResult =
|
||||
| { allowed: true }
|
||||
| { allowed: false; code: "missing-project-mapping"; reason: string };
|
||||
|
||||
export function validateProjectNodeMapping(params: {
|
||||
nodeId: string;
|
||||
mappedPath: string | undefined;
|
||||
}): NodeDispatchValidationResult {
|
||||
const { nodeId, mappedPath } = params;
|
||||
if (typeof mappedPath !== "string" || mappedPath.trim().length === 0) {
|
||||
return {
|
||||
allowed: false,
|
||||
code: "missing-project-mapping",
|
||||
reason: `Execution blocked: project has no path mapping for node ${nodeId}`,
|
||||
};
|
||||
}
|
||||
|
||||
return { allowed: true };
|
||||
}
|
||||
@@ -38,6 +38,7 @@ import { MissionAutopilot } from "../mission-autopilot.js";
|
||||
import { MissionExecutionLoop } from "../mission-execution-loop.js";
|
||||
import { TriageProcessor } from "../triage.js";
|
||||
import { EphemeralWorkerManager } from "../ephemeral-worker-manager.js";
|
||||
import { validateProjectNodeMapping } from "../node-dispatch-validation.js";
|
||||
|
||||
/**
|
||||
* InProcessRuntime runs a project within the main process.
|
||||
@@ -300,6 +301,10 @@ export class InProcessRuntime
|
||||
runtimeLog.log(`Scheduled task ${task.id}`);
|
||||
},
|
||||
onBlocked: () => {},
|
||||
validateNodeDispatch: async (nodeId) => {
|
||||
const mappedPath = await this.centralCore.getProjectNodePath(this.config.projectId, nodeId);
|
||||
return validateProjectNodeMapping({ nodeId, mappedPath });
|
||||
},
|
||||
|
||||
});
|
||||
|
||||
|
||||
@@ -19,6 +19,7 @@ import { reconcileMissionFeatureState } from "./mission-feature-sync.js";
|
||||
import { evaluateSpecStaleness, getPromptPath } from "./spec-staleness.js";
|
||||
import { resolveEffectiveNode } from "./effective-node.js";
|
||||
import { applyUnavailableNodePolicy } from "./node-routing-policy.js";
|
||||
import type { NodeDispatchValidationResult } from "./node-dispatch-validation.js";
|
||||
|
||||
/**
|
||||
* Check whether two sets of file scope paths overlap.
|
||||
@@ -135,6 +136,8 @@ export interface SchedulerOptions {
|
||||
* Reserved for FN-2722-C (unavailable node policy enforcement).
|
||||
* Accepted here so the option can be wired at construction time. */
|
||||
nodeHealthMonitor?: import("./node-health-monitor.js").NodeHealthMonitor;
|
||||
/** Optional dispatch validator used to block dispatch on configuration issues before health policy checks. */
|
||||
validateNodeDispatch?: (nodeId: string) => Promise<NodeDispatchValidationResult>;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -168,6 +171,8 @@ export class Scheduler {
|
||||
private failedTaskIds = new Set<string>();
|
||||
/** Tracks tasks blocked by unavailable-node policy to deduplicate block log entries. */
|
||||
private wasNodeBlocked = new Set<string>();
|
||||
/** Tracks tasks blocked by missing project-node mapping to deduplicate block log entries. */
|
||||
private wasNodeDispatchValidationBlocked = new Set<string>();
|
||||
|
||||
/**
|
||||
* Async listener guard convention:
|
||||
@@ -401,6 +406,7 @@ export class Scheduler {
|
||||
}
|
||||
this.failedTaskIds.clear();
|
||||
this.wasNodeBlocked.clear();
|
||||
this.wasNodeDispatchValidationBlocked.clear();
|
||||
schedulerLog.log("Stopped");
|
||||
}
|
||||
|
||||
@@ -769,6 +775,21 @@ export class Scheduler {
|
||||
let effectiveNode = resolveEffectiveNode(freshTask, settings);
|
||||
schedulerLog.log(`Task ${task.id} routed to node=${effectiveNode.nodeId ?? "local"} (source=${effectiveNode.source})`);
|
||||
|
||||
// Enforce dispatch configuration validation before node-health fallback logic.
|
||||
if (effectiveNode.nodeId !== undefined && this.options.validateNodeDispatch) {
|
||||
const validation = await this.options.validateNodeDispatch(effectiveNode.nodeId);
|
||||
if (!validation.allowed) {
|
||||
if (!this.wasNodeDispatchValidationBlocked.has(task.id)) {
|
||||
this.wasNodeDispatchValidationBlocked.add(task.id);
|
||||
schedulerLog.log(`Task ${task.id} dispatch blocked — ${validation.reason}`);
|
||||
await this.store.logEntry(task.id, validation.reason);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
this.wasNodeDispatchValidationBlocked.delete(task.id);
|
||||
}
|
||||
|
||||
// Enforce unavailable-node policy
|
||||
if (effectiveNode.nodeId !== undefined && this.options.nodeHealthMonitor) {
|
||||
const nodeHealth = this.options.nodeHealthMonitor.getNodeHealth(effectiveNode.nodeId);
|
||||
@@ -817,6 +838,7 @@ export class Scheduler {
|
||||
this.planWorktreePath(task, settings.worktreeNaming, reservedNames),
|
||||
});
|
||||
this.wasNodeBlocked.delete(task.id);
|
||||
this.wasNodeDispatchValidationBlocked.delete(task.id);
|
||||
await this.store.logEntry(task.id, `Node routing resolved: ${effectiveNode.nodeId ?? "local"} (source: ${effectiveNode.source})`);
|
||||
this.options.onSchedule?.(task);
|
||||
started++;
|
||||
|
||||
Reference in New Issue
Block a user