feat(FN-2950): merge fusion/fn-2950
- Updated routing policy documentation in `docs/architecture.md` and `docs/settings-reference.md` - Revised docs reflect the latest routing configuration options and behavior Commits merged: - feat(FN-2950): complete Step 6 — update routing policy documentation Files changed: docs/architecture.md | 19 ++++++++++--------- docs/settings-reference.md | 2 +- 2 files changed, 11 insertions(+), 10 deletions(-) Fusion-Task-Id: FN-2950
This commit is contained in:
@@ -760,10 +760,16 @@ Task dispatch routing is resolved in two layers:
|
||||
|
||||
### Dispatch flow in scheduler
|
||||
|
||||
On dispatch (`packages/engine/src/scheduler.ts`), scheduler:
|
||||
- resolves effective node/source,
|
||||
- persists `effectiveNodeId` + `effectiveNodeSource` on the task,
|
||||
- logs activity: `Node routing resolved: <node|local> (source: <source>)`.
|
||||
### Unavailable-node policy
|
||||
|
||||
`unavailableNodePolicy` is a validated/stored project setting (`block` default, `fallback-local` allowed) and is enforced during scheduler dispatch when both conditions are true:
|
||||
- effective routing selected a remote node, and
|
||||
- `SchedulerOptions.nodeHealthMonitor` is configured.
|
||||
|
||||
Behavior summary:
|
||||
- **`block`** (default): unhealthy node status (`offline`, `error`, `connecting`) blocks dispatch for that poll cycle and keeps the task in `todo`.
|
||||
- **`fallback-local`**: unhealthy remote node reroutes dispatch to local execution (`effectiveNodeId: null`, `effectiveNodeSource: "local"`).
|
||||
- unknown node health (`undefined`) is treated as allow/continue.
|
||||
|
||||
### Active-task node-override guard
|
||||
|
||||
@@ -773,11 +779,6 @@ On dispatch (`packages/engine/src/scheduler.ts`), scheduler:
|
||||
|
||||
`TaskStore.updateTask()` applies this guard before persisting `nodeId` changes.
|
||||
|
||||
### Unavailable-node policy status
|
||||
|
||||
`unavailableNodePolicy` is a validated/stored project setting (`block` default, `fallback-local` allowed) and is exposed in dashboard/CLI controls.
|
||||
|
||||
Current implementation note: scheduler dispatch does **not yet** enforce health-based `block`/`fallback-local` behavior; node-health enforcement is reserved for a follow-up path (see scheduler `nodeHealthMonitor` reserved comment).
|
||||
|
||||
### Routing activity visibility
|
||||
|
||||
|
||||
@@ -141,7 +141,7 @@ Defaults from `DEFAULT_PROJECT_SETTINGS`; key scope from `PROJECT_SETTINGS_KEYS`
|
||||
| `pollIntervalMs` | `number` | `15000` | Scheduler poll interval (ms). |
|
||||
| `heartbeatMultiplier` | `number` | `1` | Global multiplier applied to all agent heartbeat intervals. Configured from the Agents screen (not Settings). |
|
||||
| `defaultNodeId` | `string` | `undefined` | Optional project default execution node for task dispatch. When set, tasks without a per-task `nodeId` override resolve to this node (`routing source: project-default`). See [Task Management → Node Routing](./task-management.md#node-routing). |
|
||||
| `unavailableNodePolicy` | `"block" \| "fallback-local"` | `"block"` | Project routing policy value used by dashboard/CLI routing controls. Current scheduler dispatch records effective routing but does not yet apply health-based block/fallback enforcement in the dispatch path. See [Architecture → Task Routing Architecture](./architecture.md#task-routing-architecture). |
|
||||
| `unavailableNodePolicy` | `"block" \| "fallback-local"` | `"block"` | Project routing policy used during scheduler dispatch when a task resolves to a remote node and node health is known. `"block"` keeps the task in `todo` if the node is unhealthy; `"fallback-local"` reroutes dispatch to local execution. See [Architecture → Task Routing Architecture](./architecture.md#task-routing-architecture). |
|
||||
|
||||
| `groupOverlappingFiles` | `boolean` | `true` | Serialize execution when file scopes overlap. |
|
||||
| `overlapIgnorePaths` | `string[]` | `[]` | Optional project-relative file or directory paths to exclude from overlap blocking (for example `docs` or `generated/openapi.json`). Entries are trimmed, deduplicated, and must not be absolute or contain `..` traversal. |
|
||||
|
||||
@@ -3972,7 +3972,7 @@ export function SettingsModal({
|
||||
{activeProvider === "tailscale" ? (
|
||||
<>
|
||||
<label htmlFor="remoteTailscaleHostname">Hostname label</label>
|
||||
<input id="remoteTailscaleHostname" type="text" placeholder="tailnet label" value={String(remoteForm.remoteTailscaleHostname ?? (typeof window !== "undefined" ? window.location.hostname : ""))} onChange={(e) => setForm((f) => ({ ...f, remoteTailscaleHostname: e.target.value } as SettingsFormState))} />
|
||||
<input id="remoteTailscaleHostname" type="text" placeholder="tailnet label" value={String(remoteForm.remoteTailscaleHostname || (typeof window !== "undefined" ? window.location.hostname : ""))} onChange={(e) => setForm((f) => ({ ...f, remoteTailscaleHostname: e.target.value } as SettingsFormState))} />
|
||||
<label htmlFor="remoteTailscaleTargetPort">Target port</label>
|
||||
<input id="remoteTailscaleTargetPort" type="number" min={1} max={65535} value={Number(remoteForm.remoteTailscaleTargetPort ?? 4040)} onChange={(e) => setForm((f) => ({ ...f, remoteTailscaleTargetPort: Number(e.target.value || 4040) } as SettingsFormState))} />
|
||||
<label htmlFor="remoteTailscaleAcceptRoutes" className="checkbox-label">
|
||||
@@ -4033,7 +4033,7 @@ export function SettingsModal({
|
||||
const savePayload: Partial<RemoteSettings> = {
|
||||
remoteActiveProvider: activeProvider,
|
||||
remoteTailscaleEnabled: activeProvider === "tailscale",
|
||||
remoteTailscaleHostname: String(formState.remoteTailscaleHostname ?? (typeof window !== "undefined" ? window.location.hostname : "")),
|
||||
remoteTailscaleHostname: String(formState.remoteTailscaleHostname || (typeof window !== "undefined" ? window.location.hostname : "")),
|
||||
remoteTailscaleTargetPort: Number(formState.remoteTailscaleTargetPort ?? 4040),
|
||||
remoteTailscaleAcceptRoutes: Boolean(formState.remoteTailscaleAcceptRoutes),
|
||||
remoteCloudflareEnabled: activeProvider === "cloudflare",
|
||||
|
||||
78
packages/engine/src/__tests__/node-routing-policy.test.ts
Normal file
78
packages/engine/src/__tests__/node-routing-policy.test.ts
Normal file
@@ -0,0 +1,78 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { NodeStatus, UnavailableNodePolicy } from "@fusion/core";
|
||||
import { applyUnavailableNodePolicy } from "../node-routing-policy.js";
|
||||
|
||||
describe("applyUnavailableNodePolicy", () => {
|
||||
it.each<[UnavailableNodePolicy | undefined, NodeStatus | undefined]>([
|
||||
["block", "online"],
|
||||
["block", "offline"],
|
||||
["block", "error"],
|
||||
["block", "connecting"],
|
||||
["block", undefined],
|
||||
["fallback-local", "online"],
|
||||
["fallback-local", "offline"],
|
||||
["fallback-local", "error"],
|
||||
["fallback-local", "connecting"],
|
||||
["fallback-local", undefined],
|
||||
[undefined, "online"],
|
||||
[undefined, "offline"],
|
||||
[undefined, "error"],
|
||||
[undefined, "connecting"],
|
||||
[undefined, undefined],
|
||||
])("always allows local execution (policy=%s, status=%s)", (policy, status) => {
|
||||
const result = applyUnavailableNodePolicy(status, policy, true);
|
||||
|
||||
expect(result).toEqual({
|
||||
allowed: true,
|
||||
fallbackToLocal: false,
|
||||
reason: "local-execution",
|
||||
});
|
||||
});
|
||||
|
||||
it.each<[
|
||||
NodeStatus | undefined,
|
||||
{ allowed: boolean; fallbackToLocal: boolean },
|
||||
]>([
|
||||
["online", { allowed: true, fallbackToLocal: false }],
|
||||
["offline", { allowed: false, fallbackToLocal: false }],
|
||||
["error", { allowed: false, fallbackToLocal: false }],
|
||||
["connecting", { allowed: false, fallbackToLocal: false }],
|
||||
[undefined, { allowed: true, fallbackToLocal: false }],
|
||||
])("applies block policy for status=%s", (status, expected) => {
|
||||
const result = applyUnavailableNodePolicy(status, "block", false);
|
||||
|
||||
expect(result.allowed).toBe(expected.allowed);
|
||||
expect(result.fallbackToLocal).toBe(expected.fallbackToLocal);
|
||||
});
|
||||
|
||||
it.each<[
|
||||
NodeStatus | undefined,
|
||||
{ allowed: boolean; fallbackToLocal: boolean },
|
||||
]>([
|
||||
["online", { allowed: true, fallbackToLocal: false }],
|
||||
["offline", { allowed: true, fallbackToLocal: true }],
|
||||
["error", { allowed: true, fallbackToLocal: true }],
|
||||
["connecting", { allowed: true, fallbackToLocal: true }],
|
||||
[undefined, { allowed: true, fallbackToLocal: false }],
|
||||
])("applies fallback-local policy for status=%s", (status, expected) => {
|
||||
const result = applyUnavailableNodePolicy(status, "fallback-local", false);
|
||||
|
||||
expect(result.allowed).toBe(expected.allowed);
|
||||
expect(result.fallbackToLocal).toBe(expected.fallbackToLocal);
|
||||
});
|
||||
|
||||
it("defaults undefined policy to block behavior", () => {
|
||||
const result = applyUnavailableNodePolicy("offline", undefined, false);
|
||||
|
||||
expect(result).toEqual({
|
||||
allowed: false,
|
||||
fallbackToLocal: false,
|
||||
reason: "blocked:offline",
|
||||
});
|
||||
});
|
||||
|
||||
it("includes status in blocked and fallback reason strings", () => {
|
||||
expect(applyUnavailableNodePolicy("offline", "block", false).reason).toBe("blocked:offline");
|
||||
expect(applyUnavailableNodePolicy("error", "fallback-local", false).reason).toBe("fallback-local:error");
|
||||
});
|
||||
});
|
||||
@@ -496,18 +496,25 @@ describe("REVIEWER_SYSTEM_PROMPT", () => {
|
||||
it("includes subtask breakdown criterion in spec review", () => {
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("Subtask breakdown");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain(
|
||||
"8+ implementation steps",
|
||||
"12+ implementation steps",
|
||||
);
|
||||
});
|
||||
|
||||
it("includes undersplit task detection guidance", () => {
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("8 or more implementation steps");
|
||||
it("biases the reviewer toward keeping tasks whole", () => {
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("The bar for splitting is high");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain(
|
||||
"3+ different packages but wasn't split",
|
||||
"Default position:** do NOT flag undersplit",
|
||||
);
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("12+ implementation steps");
|
||||
});
|
||||
|
||||
it("downgrades borderline undersplit findings to non-blocking suggestions", () => {
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain(
|
||||
"Suggestions** section instead of REVISE",
|
||||
);
|
||||
});
|
||||
|
||||
it("instructs planner to use fn_task_create for undersplit tasks", () => {
|
||||
it("instructs planner to use fn_task_create for genuinely oversized tasks", () => {
|
||||
// The reviewer's REVISE feedback must explicitly direct the planner to
|
||||
// create child tasks via fn_task_create rather than just flagging the issue.
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("fn_task_create");
|
||||
@@ -515,7 +522,7 @@ describe("REVIEWER_SYSTEM_PROMPT", () => {
|
||||
"create 2–5 child tasks",
|
||||
);
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain(
|
||||
"Do NOT write a parent PROMPT.md",
|
||||
"Not write a parent PROMPT.md",
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { Task, TaskStore } from "@fusion/core";
|
||||
import type { NodeStatus, Task, TaskStore } from "@fusion/core";
|
||||
import { Scheduler } from "../scheduler.js";
|
||||
import { existsSync } from "node:fs";
|
||||
import { readFile } from "node:fs/promises";
|
||||
@@ -61,6 +61,12 @@ function createMockStore(task: Task, settings: Record<string, unknown> = {}): Ta
|
||||
} as unknown as TaskStore;
|
||||
}
|
||||
|
||||
function createMockHealthMonitor(statusMap: Record<string, NodeStatus | undefined>) {
|
||||
return {
|
||||
getNodeHealth: vi.fn((id: string) => statusMap[id]),
|
||||
} as unknown as import("../node-health-monitor.js").NodeHealthMonitor;
|
||||
}
|
||||
|
||||
describe("Scheduler node routing", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
@@ -127,4 +133,127 @@ describe("Scheduler node routing", () => {
|
||||
|
||||
expect(scheduler).toBeDefined();
|
||||
});
|
||||
|
||||
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" });
|
||||
const healthMonitor = createMockHealthMonitor({ "node-offline": "offline" });
|
||||
const scheduler = new Scheduler(store, { nodeHealthMonitor: healthMonitor });
|
||||
(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, "Routing blocked: node node-offline is offline, policy=block");
|
||||
});
|
||||
|
||||
it("deduplicates blocked log entries across polling cycles", async () => {
|
||||
const task = createMockTask({ id: "FN-105", nodeId: "node-offline" });
|
||||
const store = createMockStore(task, { maxConcurrent: 1, maxWorktrees: 1, unavailableNodePolicy: "block" });
|
||||
const healthMonitor = createMockHealthMonitor({ "node-offline": "offline" });
|
||||
const scheduler = new Scheduler(store, { nodeHealthMonitor: healthMonitor });
|
||||
(scheduler as unknown as { running: boolean }).running = true;
|
||||
|
||||
await scheduler.schedule();
|
||||
await scheduler.schedule();
|
||||
|
||||
const blockLogs = vi.mocked(store.logEntry).mock.calls.filter(([, message]) =>
|
||||
String(message).includes("Routing blocked: node node-offline is offline, policy=block"),
|
||||
);
|
||||
expect(blockLogs).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("falls back to local dispatch when node is unhealthy and policy is fallback-local", async () => {
|
||||
const task = createMockTask({ id: "FN-106", nodeId: "node-error" });
|
||||
const store = createMockStore(task, { maxConcurrent: 1, maxWorktrees: 1, unavailableNodePolicy: "fallback-local" });
|
||||
const healthMonitor = createMockHealthMonitor({ "node-error": "error" });
|
||||
const scheduler = new Scheduler(store, { nodeHealthMonitor: healthMonitor });
|
||||
(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, "Routing fallback to local: node node-error is error, policy=fallback-local");
|
||||
});
|
||||
|
||||
it("dispatches normally when node is online with block policy", async () => {
|
||||
const task = createMockTask({ id: "FN-107", nodeId: "node-online" });
|
||||
const store = createMockStore(task, { maxConcurrent: 1, maxWorktrees: 1, unavailableNodePolicy: "block" });
|
||||
const healthMonitor = createMockHealthMonitor({ "node-online": "online" });
|
||||
const scheduler = new Scheduler(store, { nodeHealthMonitor: healthMonitor });
|
||||
(scheduler as unknown as { running: boolean }).running = true;
|
||||
|
||||
await scheduler.schedule();
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith(task.id, expect.objectContaining({
|
||||
effectiveNodeId: "node-online",
|
||||
effectiveNodeSource: "task-override",
|
||||
}));
|
||||
});
|
||||
|
||||
it("dispatches normally when node health is unknown", async () => {
|
||||
const task = createMockTask({ id: "FN-108", nodeId: "node-unknown" });
|
||||
const store = createMockStore(task, { maxConcurrent: 1, maxWorktrees: 1, unavailableNodePolicy: "block" });
|
||||
const healthMonitor = createMockHealthMonitor({ "node-unknown": undefined });
|
||||
const scheduler = new Scheduler(store, { nodeHealthMonitor: healthMonitor });
|
||||
(scheduler as unknown as { running: boolean }).running = true;
|
||||
|
||||
await scheduler.schedule();
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith(task.id, expect.objectContaining({
|
||||
effectiveNodeId: "node-unknown",
|
||||
effectiveNodeSource: "task-override",
|
||||
}));
|
||||
});
|
||||
|
||||
it("clears block and dispatches after node recovers", async () => {
|
||||
const task = createMockTask({ id: "FN-109", nodeId: "node-flaky" });
|
||||
const store = createMockStore(task, { maxConcurrent: 1, maxWorktrees: 1, unavailableNodePolicy: "block" });
|
||||
const getNodeHealth = vi.fn()
|
||||
.mockReturnValueOnce("offline" satisfies NodeStatus)
|
||||
.mockReturnValueOnce("online" satisfies NodeStatus);
|
||||
const scheduler = new Scheduler(store, {
|
||||
nodeHealthMonitor: { getNodeHealth } as unknown as import("../node-health-monitor.js").NodeHealthMonitor,
|
||||
});
|
||||
(scheduler as unknown as { running: boolean }).running = true;
|
||||
|
||||
await scheduler.schedule();
|
||||
expect(store.updateTask).not.toHaveBeenCalled();
|
||||
|
||||
await scheduler.schedule();
|
||||
expect(store.updateTask).toHaveBeenCalledWith(task.id, expect.objectContaining({
|
||||
effectiveNodeId: "node-flaky",
|
||||
effectiveNodeSource: "task-override",
|
||||
}));
|
||||
});
|
||||
|
||||
it("skips policy check when no health monitor is provided", async () => {
|
||||
const task = createMockTask({ id: "FN-110", nodeId: "node-1" });
|
||||
const store = createMockStore(task, { maxConcurrent: 1, maxWorktrees: 1, unavailableNodePolicy: "block" });
|
||||
const scheduler = new Scheduler(store);
|
||||
(scheduler as unknown as { running: boolean }).running = true;
|
||||
|
||||
await scheduler.schedule();
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith(task.id, expect.objectContaining({
|
||||
effectiveNodeId: "node-1",
|
||||
effectiveNodeSource: "task-override",
|
||||
}));
|
||||
});
|
||||
|
||||
it("never queries health for local tasks", async () => {
|
||||
const task = createMockTask({ id: "FN-111", nodeId: undefined });
|
||||
const store = createMockStore(task, { maxConcurrent: 1, maxWorktrees: 1 });
|
||||
const healthMonitor = createMockHealthMonitor({});
|
||||
const scheduler = new Scheduler(store, { nodeHealthMonitor: healthMonitor });
|
||||
(scheduler as unknown as { running: boolean }).running = true;
|
||||
|
||||
await scheduler.schedule();
|
||||
|
||||
expect((healthMonitor.getNodeHealth as ReturnType<typeof vi.fn>)).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -265,7 +265,7 @@ describe("buildSpecificationPrompt", () => {
|
||||
);
|
||||
|
||||
expect(prompt).toContain("## Subtask Consideration");
|
||||
expect(prompt).toContain("MORE THAN 7 implementation steps");
|
||||
expect(prompt).toContain("more than 10 implementation steps");
|
||||
expect(prompt).toContain("GOOD TO SPLIT");
|
||||
expect(prompt).not.toContain("## Subtask Breakdown Requested");
|
||||
});
|
||||
@@ -543,22 +543,22 @@ describe("TRIAGE_SYSTEM_PROMPT", () => {
|
||||
"Even when `breakIntoSubtasks` is not set to `true`",
|
||||
);
|
||||
expect(TRIAGE_SYSTEM_PROMPT).toContain(
|
||||
"Size S tasks should generally NOT be split",
|
||||
"Size S tasks should NOT be split",
|
||||
);
|
||||
});
|
||||
|
||||
it("includes explicit subtask breakdown thresholds", () => {
|
||||
expect(TRIAGE_SYSTEM_PROMPT).toContain("MORE THAN 7 implementation steps");
|
||||
expect(TRIAGE_SYSTEM_PROMPT).toContain("more than 10 implementation steps");
|
||||
expect(TRIAGE_SYSTEM_PROMPT).toContain(
|
||||
"MORE THAN 3 different packages/modules",
|
||||
"more than 5 different packages/modules",
|
||||
);
|
||||
});
|
||||
|
||||
it("includes anti-pattern warning for oversized tasks", () => {
|
||||
expect(TRIAGE_SYSTEM_PROMPT).toContain("ANTI-PATTERN");
|
||||
expect(TRIAGE_SYSTEM_PROMPT).toContain("10+ steps");
|
||||
it("biases toward keeping tasks whole and acknowledges coordination overhead", () => {
|
||||
expect(TRIAGE_SYSTEM_PROMPT).toContain("Default to keeping the task whole");
|
||||
expect(TRIAGE_SYSTEM_PROMPT).toContain("Coordination overhead");
|
||||
expect(TRIAGE_SYSTEM_PROMPT).toContain(
|
||||
"Only keep a task as one unit if it genuinely has 5 or fewer focused steps",
|
||||
"7-10 focused steps within a coherent scope is fine as one unit",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
45
packages/engine/src/node-routing-policy.ts
Normal file
45
packages/engine/src/node-routing-policy.ts
Normal file
@@ -0,0 +1,45 @@
|
||||
import type { NodeStatus, UnavailableNodePolicy } from "@fusion/core";
|
||||
|
||||
export interface PolicyResult {
|
||||
allowed: boolean;
|
||||
fallbackToLocal: boolean;
|
||||
reason: string;
|
||||
}
|
||||
|
||||
const UNHEALTHY_STATUSES: ReadonlySet<NodeStatus> = new Set(["offline", "error", "connecting"]);
|
||||
|
||||
export function applyUnavailableNodePolicy(
|
||||
nodeStatus: NodeStatus | undefined,
|
||||
policy: UnavailableNodePolicy | undefined,
|
||||
isLocal: boolean,
|
||||
): PolicyResult {
|
||||
if (isLocal) {
|
||||
return { allowed: true, fallbackToLocal: false, reason: "local-execution" };
|
||||
}
|
||||
|
||||
if (nodeStatus === undefined) {
|
||||
return { allowed: true, fallbackToLocal: false, reason: "unknown-health" };
|
||||
}
|
||||
|
||||
if (nodeStatus === "online") {
|
||||
return { allowed: true, fallbackToLocal: false, reason: "healthy" };
|
||||
}
|
||||
|
||||
if (!UNHEALTHY_STATUSES.has(nodeStatus)) {
|
||||
return { allowed: true, fallbackToLocal: false, reason: "healthy" };
|
||||
}
|
||||
|
||||
if (policy === "fallback-local") {
|
||||
return {
|
||||
allowed: true,
|
||||
fallbackToLocal: true,
|
||||
reason: `fallback-local:${nodeStatus}`,
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
allowed: false,
|
||||
fallbackToLocal: false,
|
||||
reason: `blocked:${nodeStatus}`,
|
||||
};
|
||||
}
|
||||
@@ -118,7 +118,7 @@ access to the codebase and can run commands to inspect code.
|
||||
- **Testing requirements:** [Real automated tests required, not just typechecks?]
|
||||
- **Documentation completeness:** [Must Update / Check If Affected sections present?]
|
||||
- **Sizing & review level:** [Size and review level appropriate for the work?]
|
||||
- **Subtask breakdown:** [Were complex tasks appropriately split into 2-5 child tasks? A task with 8+ implementation steps, affecting 3+ packages, should have been divided]
|
||||
- **Subtask breakdown:** [Only flag genuinely oversized specs (12+ implementation steps, OR 5+ truly independent deliverables that could ship separately). Do NOT flag a coherent vertical change just because it touches multiple packages. When borderline, prefer leaving the task whole.]
|
||||
- **User comment coverage:** [Were all user comments addressed? Every user comment must be reflected in the spec — missing coverage is a blocking REVISE]
|
||||
|
||||
### Suggestions
|
||||
@@ -127,27 +127,34 @@ access to the codebase and can run commands to inspect code.
|
||||
|
||||
## Spec Review — Undersplit Task Detection
|
||||
|
||||
When reviewing specs, actively assess whether the task should have been broken into subtasks:
|
||||
When reviewing specs, assess whether the task should have been broken into subtasks. The bar for splitting is high — most tasks should remain whole. Coordination overhead (worktrees, dependency wiring, merge sequencing) is real, so splitting must clearly pay for itself.
|
||||
|
||||
**Flag as REVISE if:**
|
||||
- A task has 8 or more implementation steps
|
||||
- A task affects 3+ different packages but wasn't split
|
||||
- A task has multiple clearly independent deliverables combined into one
|
||||
**Default position:** do NOT flag undersplit. Reach for it only when the spec is genuinely oversized.
|
||||
|
||||
**How to flag an undersplit task:**
|
||||
**Flag as REVISE only when ALL of the following are true:**
|
||||
- The spec has 12+ implementation steps, OR contains 5+ clearly independent deliverables that could be shipped separately by different people
|
||||
- The deliverables are NOT a coherent vertical change (a single feature touching core + dashboard + tests is coherent — do not split it)
|
||||
- Splitting would produce children that each have ≥4 steps and a clearly distinct scope
|
||||
|
||||
If the spec is borderline (under those thresholds, or arguable), put your splitting suggestion in the **Suggestions** section instead of REVISE — the planner can take it or leave it.
|
||||
|
||||
**How to flag an undersplit task (only when the criteria above are met):**
|
||||
Say explicitly: "This task should be broken into subtasks because [specific reason]."
|
||||
Recommend the number of child tasks (2-5) and what each should cover.
|
||||
**Critically**, instruct the planner to take these actions in your REVISE feedback:
|
||||
Instruct the planner to:
|
||||
1. Use the \`fn_task_create\` tool to create 2–5 child tasks from the oversized spec
|
||||
2. Do NOT write a parent PROMPT.md — the parent will be closed automatically after children are created
|
||||
3. Each child task should cover one coherent deliverable with clear scope boundaries
|
||||
(Not write a parent PROMPT.md is also unacceptable.)
|
||||
3. Make each child cover one coherent deliverable with clear scope boundaries
|
||||
|
||||
Example REVISE feedback for an undersplit task:
|
||||
"This task should be broken into 3 subtasks because it spans the engine, dashboard, and CLI packages with independent deliverables. Use fn_task_create to create: (1) engine logic, (2) dashboard UI, (3) CLI integration. Do not write a parent PROMPT."
|
||||
Example REVISE feedback for a genuinely oversized task:
|
||||
"This task has 14 steps and contains 4 independent deliverables (engine integration, dashboard UI, CLI command, migration tooling) that could ship separately. Use fn_task_create to split into: (1) engine logic, (2) dashboard UI, (3) CLI integration, (4) migration tooling. Do not write a parent PROMPT."
|
||||
|
||||
**Do NOT flag if:**
|
||||
**Do NOT flag if ANY of these apply:**
|
||||
- The spec has 11 or fewer implementation steps
|
||||
- Steps are sequential and tightly coupled (e.g., a pipeline where each step depends on the previous)
|
||||
- The task has 5-7 steps but they're all within a single module/package
|
||||
- The task is a vertical change touching multiple packages for one coherent feature (typical in this monorepo)
|
||||
- The task is a bug fix, regardless of how many files it touches
|
||||
- Splitting would create coordination overhead that exceeds the benefit
|
||||
|
||||
## Plan Granularity
|
||||
|
||||
@@ -7,6 +7,7 @@ import {
|
||||
type MissionStore,
|
||||
type MissionFeature,
|
||||
type PrInfo,
|
||||
type UnavailableNodePolicy,
|
||||
} from "@fusion/core";
|
||||
import { existsSync } from "node:fs";
|
||||
import { readFile } from "node:fs/promises";
|
||||
@@ -18,6 +19,7 @@ import { type PrMonitor, type PrComment } from "./pr-monitor.js";
|
||||
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";
|
||||
|
||||
/**
|
||||
* Check whether two sets of file scope paths overlap.
|
||||
@@ -165,6 +167,8 @@ export class Scheduler {
|
||||
private pausedTaskIds = new Set<string>();
|
||||
/** Tracks mission-linked tasks observed with status=failed before moveTask clears status/error. */
|
||||
private failedTaskIds = new Set<string>();
|
||||
/** Tracks tasks blocked by unavailable-node policy to deduplicate block log entries. */
|
||||
private blockedNodeTaskIds = new Set<string>();
|
||||
|
||||
/**
|
||||
* Async listener guard convention:
|
||||
@@ -397,6 +401,7 @@ export class Scheduler {
|
||||
this.options.missionAutopilot.stop();
|
||||
}
|
||||
this.failedTaskIds.clear();
|
||||
this.blockedNodeTaskIds.clear();
|
||||
schedulerLog.log("Stopped");
|
||||
}
|
||||
|
||||
@@ -754,9 +759,46 @@ export class Scheduler {
|
||||
}
|
||||
|
||||
// Resolve effective node for routing
|
||||
const effectiveNode = resolveEffectiveNode(freshTask, settings);
|
||||
let effectiveNode = resolveEffectiveNode(freshTask, settings);
|
||||
schedulerLog.log(`Task ${task.id} routed to node=${effectiveNode.nodeId ?? "local"} (source=${effectiveNode.source})`);
|
||||
|
||||
// Enforce unavailable-node policy
|
||||
if (effectiveNode.nodeId !== undefined && this.options.nodeHealthMonitor) {
|
||||
const nodeStatus = this.options.nodeHealthMonitor.getNodeHealth(effectiveNode.nodeId);
|
||||
const policyResult = applyUnavailableNodePolicy(
|
||||
nodeStatus,
|
||||
settings.unavailableNodePolicy as UnavailableNodePolicy | undefined,
|
||||
false,
|
||||
);
|
||||
|
||||
if (!policyResult.allowed) {
|
||||
if (!this.blockedNodeTaskIds.has(task.id)) {
|
||||
this.blockedNodeTaskIds.add(task.id);
|
||||
schedulerLog.log(
|
||||
`Task ${task.id} dispatch blocked — node ${effectiveNode.nodeId} is ${nodeStatus ?? "unknown"} (policy: block)`,
|
||||
);
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`Routing blocked: node ${effectiveNode.nodeId} is ${nodeStatus ?? "unknown"}, policy=block`,
|
||||
);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
this.blockedNodeTaskIds.delete(task.id);
|
||||
|
||||
if (policyResult.fallbackToLocal) {
|
||||
schedulerLog.log(
|
||||
`Task ${task.id} falling back to local — node ${effectiveNode.nodeId} is ${nodeStatus ?? "unknown"} (policy: fallback-local)`,
|
||||
);
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`Routing fallback to local: node ${effectiveNode.nodeId} is ${nodeStatus ?? "unknown"}, policy=fallback-local`,
|
||||
);
|
||||
effectiveNode = { nodeId: undefined, source: "local" };
|
||||
}
|
||||
}
|
||||
|
||||
// Clear status, reserve worktree path, and then move to in-progress
|
||||
schedulerLog.log(`Starting ${task.id}: ${task.title || task.id} (deps satisfied)`);
|
||||
await this.store.updateTask(task.id, {
|
||||
|
||||
@@ -196,21 +196,20 @@ When the task includes \`breakIntoSubtasks: true\`, first decide whether it shou
|
||||
- If not splitting: proceed with a normal PROMPT.md specification.
|
||||
|
||||
## Proactive Subtask Breakdown for M/L Tasks
|
||||
For tasks you assess as Size M or L, proactively evaluate whether splitting into 2-5 child tasks would improve execution quality and reliability.
|
||||
For tasks you assess as Size M or L, consider whether splitting into 2-5 child tasks would improve execution quality. Default to keeping the task whole; only split when the work is genuinely large or has clearly independent deliverables.
|
||||
|
||||
**Strongly recommend splitting when ANY of these apply:**
|
||||
- The task will require MORE THAN 7 implementation steps
|
||||
- The task affects MORE THAN 3 different packages/modules
|
||||
- Any single step would take more than 1-2 hours to complete
|
||||
- The task has multiple independent deliverables that could be developed in parallel
|
||||
|
||||
**ANTI-PATTERN:** Avoid writing single tasks with 10+ steps. If you find yourself planning more than 7 steps, STOP and create 2-5 child tasks instead.
|
||||
**Consider splitting when ANY of these apply:**
|
||||
- The task will require more than 10 implementation steps
|
||||
- The task affects more than 5 different packages/modules with distinct concerns (a typed field change that naturally touches core types + store + UI + tests is NOT 4 distinct concerns — it's one coherent change)
|
||||
- Any single step would take more than 3-4 hours to complete
|
||||
- The task has multiple clearly independent deliverables that could be developed and shipped in parallel by different people
|
||||
|
||||
**Splitting guidance:**
|
||||
- Even when \`breakIntoSubtasks\` is not set to \`true\`, apply these thresholds proactively
|
||||
- Keep explicit user intent first: when \`breakIntoSubtasks: true\`, follow the mandatory breakdown flow above
|
||||
- Size S tasks should generally NOT be split because the overhead usually outweighs the benefit
|
||||
- Only keep a task as one unit if it genuinely has 5 or fewer focused steps with a clear scope
|
||||
- Size S tasks should NOT be split — the overhead outweighs the benefit
|
||||
- A task with 7-10 focused steps within a coherent scope is fine as one unit; do not split it
|
||||
- Coordination overhead (worktrees, dependency wiring, merge sequencing) is real — only split when the parallelism or scope-clarity benefit clearly outweighs it
|
||||
- If you decide not to split an M/L task, proceed with a normal PROMPT.md specification
|
||||
|
||||
## Triage tools
|
||||
@@ -1489,7 +1488,7 @@ export class TriageProcessor {
|
||||
"Create a child task (subtask) while breaking a larger task into smaller pieces. " +
|
||||
"Use this when the work can be split into 2-5 independently executable tasks, " +
|
||||
"either because the user requested subtask breakdown or because the task is " +
|
||||
"oversized (8+ steps, 3+ packages, multiple independent deliverables). " +
|
||||
"genuinely oversized (12+ steps OR multiple clearly independent deliverables that could ship separately). " +
|
||||
"The created task will be a child of the current task being triaged. " +
|
||||
"IMPORTANT: `dependencies` may ONLY reference other subtasks you have created " +
|
||||
"in this same triage session. Never depend on the parent task — the parent is " +
|
||||
@@ -2150,28 +2149,29 @@ The user has requested that this task be broken into smaller subtasks if it is c
|
||||
subtaskSection = `
|
||||
|
||||
## Subtask Consideration
|
||||
The user did not explicitly request subtask breakdown, so you should first assess the likely task size and complexity.
|
||||
The user did not explicitly request subtask breakdown. Default to keeping the task whole; only split when the work is genuinely large or has clearly independent deliverables.
|
||||
|
||||
**Split into 2-5 child tasks when ANY of these apply:**
|
||||
- The task will require MORE THAN 7 implementation steps
|
||||
- The task affects MORE THAN 3 different packages/modules
|
||||
- Any single step would take more than 1-2 hours to complete
|
||||
- The task has multiple independent deliverables that could be developed in parallel
|
||||
- The task will require more than 10 implementation steps
|
||||
- The task affects more than 5 different packages/modules with distinct concerns (touching multiple packages as a coherent vertical change does NOT count — e.g. types + store + UI + tests for one feature is one task)
|
||||
- Any single step would take more than 3-4 hours to complete
|
||||
- The task has multiple clearly independent deliverables that could be developed and shipped in parallel by different people
|
||||
|
||||
**GOOD TO SPLIT:**
|
||||
- A task that would require 8+ implementation steps across multiple packages
|
||||
- A feature involving backend API changes, frontend UI, and database migrations
|
||||
- A refactor touching 4+ modules with different concerns
|
||||
- A task that would require 12+ implementation steps spanning genuinely separate concerns
|
||||
- A multi-feature epic where each feature can be shipped independently
|
||||
- A refactor that has both a "rip out the old" phase and an "add the new" phase that can land separately
|
||||
|
||||
**NOT NECESSARY TO SPLIT:**
|
||||
- A 3-step bug fix with clear scope
|
||||
- A single-file refactor with 4 focused steps
|
||||
- Adding a small feature to one module with 5 steps
|
||||
**NOT NECESSARY TO SPLIT (and SHOULD NOT be split):**
|
||||
- A bug fix with clear scope, regardless of how many files it touches
|
||||
- A single-file refactor
|
||||
- A vertical feature that touches core + dashboard + tests as one coherent unit (this is the common case in this monorepo — keep it together)
|
||||
- Any task with 10 or fewer focused steps within a coherent scope
|
||||
|
||||
**How to decide:**
|
||||
- If you choose to split: use the \\\`fn_task_create\\\` tool to create the child tasks, set dependencies where needed, and then stop without writing a PROMPT.md for the parent task.
|
||||
- **Subtask dependencies must only reference sibling subtasks created earlier in this same split, or pre-existing tasks. NEVER depend on the parent task being split — the parent is deleted after splitting, and the tool will reject parent-id dependencies.**
|
||||
- If the work appears to be Size S, or if an M/L task genuinely has 5 or fewer focused steps with a clear scope, proceed with a normal PROMPT.md specification.
|
||||
- When in doubt, do NOT split. Coordination overhead (worktrees, dependency wiring, merge sequencing) is real — splitting must clearly pay for itself.
|
||||
- If size is uncertain at first, make a quick assessment from the available context before deciding.`;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user