FN-8490: load skills for foreach step-execute sessions
Honor skill-executor configuration for implementation sessions created by foreach templates. - Propagate validated step-execute skill names through workflow seam context. - Load namespaced and bare skills with configured discovery paths for pinned step sessions. - Add regression coverage, workflow documentation, and a minor changeset. Files changed: .changeset/fn-8490-step-execute-skill.md | 7 ++ docs/workflow-steps.md | 4 +- .../__tests__/step-execute-skill-loading.test.ts | 128 +++++++++++++++++++++ packages/engine/src/executor.ts | 62 +++++++++- packages/engine/src/workflow-node-handlers.ts | 21 ++++ 5 files changed, 219 insertions(+), 3 deletions(-) Fusion-Task-Id: FN-8490 Fusion-Task-Lineage: aa1ff02d-3139-45f2-8853-f53c0aef0f2f Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-8490-step-execute-skill.md
Normal file
7
.changeset/fn-8490-step-execute-skill.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": minor
|
||||
---
|
||||
|
||||
summary: Honor skill-executor config on foreach step-execute sessions so per-step skills load like top-level nodes.
|
||||
category: feature
|
||||
dev: Threads config.executor/skillName from step-execute into StepSessionExecutor requestedSkillNames + additionalSkillPaths with FN-8461 skill-load parity (issue #2402).
|
||||
@@ -92,7 +92,9 @@ FNXC:WorkflowSteps 2026-08-08-00:00:
|
||||
FN-7145 locks the execution invariant for skill-backed workflow nodes: naming a skill must load the skill into the step session, not only mention it in prompt text. FN-8461 / GitHub #2388 makes discovery multi-source: enabled-plugin body directories and the optional Compound Engineering root both participate. The executor warns only when the named skill has no viable source after that merge; missing CE configuration alone must not mislead operators when a plugin body resolves the named skill.
|
||||
-->
|
||||
|
||||
Skill-backed prompt/gate nodes run through the same workflow-step session builder as other prompt nodes, but their `skillName` is also treated as a resource-loading request. At execution time Fusion merges both the namespaced form (for example `compound-engineering:ce-work`) and the bare form (`ce-work`) into `requestedSkillNames`. Discovery paths are merged from enabled-plugin skill body directories and, when configured, the injected `FUSION_CE_SKILLS_DIR` Compound Engineering install root.
|
||||
Skill-backed prompt/gate nodes run through the same workflow-step session builder as other prompt nodes, but their `config.skillName` is also treated as a resource-loading request when `config.executor` is `"skill"`. These are fields on the node's `config` bag; author-facing shorthand such as `executor: "skill"` refers to `config.executor`, not a root node property. At execution time Fusion merges both the namespaced form (for example `compound-engineering:ce-work`) and the bare form (`ce-work`) into `requestedSkillNames`. Discovery paths are merged from enabled-plugin skill body directories and, when configured, the injected `FUSION_CE_SKILLS_DIR` Compound Engineering install root.
|
||||
|
||||
The same contract applies to a `config.seam: "step-execute"` node inside a `foreach` template. Its per-instance implementation session receives the named skill request, while the shared implementation pass keeps the template's skill pin for its lifetime. Missing skills degrade exactly like top-level skill nodes: Fusion logs `[skill-load]` after multi-source discovery and continues with role-fallback skills rather than failing solely because a body is unavailable.
|
||||
|
||||
The executor logs `[skill-load]` only when the **named** skill has no viable discovery source after that multi-source merge. Therefore an unset optional CE directory does not warn when the requested plugin skill body is discoverable, and paths for an unrelated skill do not suppress a missing-name warning. CE-root-dependent skills still require `FUSION_CE_SKILLS_DIR` when no plugin or other source delivers their body; absence of every viable source remains a loud warning rather than a silent role-skill fallback.
|
||||
|
||||
|
||||
128
packages/engine/src/__tests__/step-execute-skill-loading.test.ts
Normal file
128
packages/engine/src/__tests__/step-execute-skill-loading.test.ts
Normal file
@@ -0,0 +1,128 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import "./executor-test-helpers.js";
|
||||
import { TaskExecutor } from "../executor.js";
|
||||
import {
|
||||
createMockStore,
|
||||
mockedStepSessionExecutor,
|
||||
resetExecutorMocks,
|
||||
} from "./executor-test-helpers.js";
|
||||
import type { WorkflowIrNode } from "@fusion/core";
|
||||
import {
|
||||
createPrimitivePromptLikeHandler,
|
||||
createPromptLikeHandler,
|
||||
FOREACH_ACTIVE_CONTEXT_KEY,
|
||||
SEAM_SKILL_NAME_CONTEXT_KEY,
|
||||
type WorkflowLegacySeams,
|
||||
} from "../workflow-node-handlers.js";
|
||||
|
||||
const task = { id: "FN-8490", title: "Skill seam", steps: [] } as any;
|
||||
const active = { foreachNodeId: "foreach", stepIndex: 0, instanceId: "foreach#0" };
|
||||
|
||||
function skillNode(config: Record<string, unknown>): WorkflowIrNode {
|
||||
return { id: "step-execute", kind: "prompt", config: { seam: "step-execute", ...config } };
|
||||
}
|
||||
|
||||
function legacySeams(stepExecute: WorkflowLegacySeams["stepExecute"]): WorkflowLegacySeams {
|
||||
const ok = async () => ({ outcome: "success" as const });
|
||||
return { planning: ok, execute: ok, review: ok, merge: ok, schedule: ok, stepExecute };
|
||||
}
|
||||
|
||||
/**
|
||||
* FNXC:WorkflowStepSkills 2026-07-22-00:00:
|
||||
* FN-8490 regression coverage uses production's config-bag executor fields. A
|
||||
* root-level executor must never create a resource pin because it is not IR.
|
||||
*/
|
||||
describe("foreach step-execute skill loading context", () => {
|
||||
it("stamps a trimmed config skill request for the legacy prompt-like seam", async () => {
|
||||
const stepExecute = vi.fn(async () => ({ outcome: "success" as const }));
|
||||
const handler = createPromptLikeHandler(legacySeams(stepExecute));
|
||||
const context = { [FOREACH_ACTIVE_CONTEXT_KEY]: active };
|
||||
|
||||
await handler(skillNode({ executor: "skill", skillName: " verify " }), { task, context } as any);
|
||||
|
||||
expect(context[SEAM_SKILL_NAME_CONTEXT_KEY]).toBe("verify");
|
||||
expect(stepExecute).toHaveBeenCalledWith(task, expect.objectContaining({
|
||||
[SEAM_SKILL_NAME_CONTEXT_KEY]: "verify",
|
||||
}));
|
||||
});
|
||||
|
||||
it.each([
|
||||
[{ executor: "model", skillName: "verify" }],
|
||||
[{ executor: "skill", skillName: " " }],
|
||||
[{ skillName: "verify" }],
|
||||
])("does not invent a skill request for %o", async (config) => {
|
||||
const handler = createPromptLikeHandler(legacySeams(async () => ({ outcome: "success" })));
|
||||
const context: Record<string, unknown> = {
|
||||
[FOREACH_ACTIVE_CONTEXT_KEY]: active,
|
||||
[SEAM_SKILL_NAME_CONTEXT_KEY]: "stale-skill",
|
||||
};
|
||||
|
||||
await handler(skillNode(config), { task, context } as any);
|
||||
|
||||
expect(context).not.toHaveProperty(SEAM_SKILL_NAME_CONTEXT_KEY);
|
||||
});
|
||||
|
||||
it("stamps the same config-bag skill request for the primitive step-execute path", async () => {
|
||||
const runTaskStep = vi.fn(async () => ({ outcome: "success" as const }));
|
||||
const handler = createPrimitivePromptLikeHandler({ runTaskStep } as any);
|
||||
const context: Record<string, unknown> = { [FOREACH_ACTIVE_CONTEXT_KEY]: active };
|
||||
|
||||
await handler(skillNode({ executor: "skill", skillName: "security-scan" }), { task, context } as any);
|
||||
|
||||
expect(context[SEAM_SKILL_NAME_CONTEXT_KEY]).toBe("security-scan");
|
||||
expect(runTaskStep).toHaveBeenCalledWith(expect.anything(), task, 0);
|
||||
});
|
||||
});
|
||||
|
||||
function stepSessionTask() {
|
||||
return {
|
||||
id: "FN-8490", title: "Skill seam", description: "Skill seam", column: "in-progress",
|
||||
dependencies: [], steps: [{ name: "Implement", status: "pending" }], currentStep: 0,
|
||||
prompt: "# test\n## Steps\n### Step 0: Implement\n- [ ] implement",
|
||||
createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(),
|
||||
};
|
||||
}
|
||||
|
||||
describe("foreach step-execute skill session selection", () => {
|
||||
beforeEach(() => resetExecutorMocks());
|
||||
|
||||
it("merges namespaced and bare requested names plus the CE discovery root", async () => {
|
||||
const store = createMockStore();
|
||||
const taskDetail = stepSessionTask();
|
||||
store.getTask.mockResolvedValue(taskDetail as any);
|
||||
const executor = new TaskExecutor(store as any, "/tmp/test", { agentStore: { getAgent: vi.fn() } } as any);
|
||||
(executor as any).graphStepSessionPinned.add(taskDetail.id);
|
||||
(executor as any).graphSeamSkillName.set(taskDetail.id, "compound-engineering:verify");
|
||||
|
||||
const previousCeSkillsDir = process.env.FUSION_CE_SKILLS_DIR;
|
||||
process.env.FUSION_CE_SKILLS_DIR = "/opt/ce/.fusion-ce-skills";
|
||||
try {
|
||||
await (executor as any).runImplementationPhase(taskDetail);
|
||||
} finally {
|
||||
if (previousCeSkillsDir === undefined) delete process.env.FUSION_CE_SKILLS_DIR;
|
||||
else process.env.FUSION_CE_SKILLS_DIR = previousCeSkillsDir;
|
||||
}
|
||||
|
||||
const options = mockedStepSessionExecutor.mock.calls.at(-1)?.[0] as any;
|
||||
expect(options.skillSelection.requestedSkillNames).toEqual(expect.arrayContaining([
|
||||
"compound-engineering:verify", "verify",
|
||||
]));
|
||||
expect(options.additionalSkillPaths).toContain("/opt/ce/.fusion-ce-skills");
|
||||
expect(store.logEntry).not.toHaveBeenCalledWith(expect.anything(), expect.stringContaining("[skill-load]"));
|
||||
});
|
||||
|
||||
it("warns but retains role-fallback selection when the pinned skill is missing", async () => {
|
||||
const store = createMockStore();
|
||||
const taskDetail = stepSessionTask();
|
||||
store.getTask.mockResolvedValue(taskDetail as any);
|
||||
const executor = new TaskExecutor(store as any, "/tmp/test", { agentStore: { getAgent: vi.fn() } } as any);
|
||||
(executor as any).graphStepSessionPinned.add(taskDetail.id);
|
||||
(executor as any).graphSeamSkillName.set(taskDetail.id, "missing-verify");
|
||||
|
||||
await (executor as any).runImplementationPhase(taskDetail);
|
||||
|
||||
const options = mockedStepSessionExecutor.mock.calls.at(-1)?.[0] as any;
|
||||
expect(options.skillSelection.requestedSkillNames).toContain("missing-verify");
|
||||
expect(store.logEntry).toHaveBeenCalledWith(taskDetail.id, expect.stringContaining("[skill-load] Foreach step-execute"));
|
||||
});
|
||||
});
|
||||
@@ -35,6 +35,7 @@ import type {
|
||||
import {
|
||||
FOREACH_ACTIVE_CONTEXT_KEY,
|
||||
SEAM_GOVERNING_NODE_CONTEXT_KEY,
|
||||
SEAM_SKILL_NAME_CONTEXT_KEY,
|
||||
SEAM_THINKING_LEVEL_CONTEXT_KEY,
|
||||
SPLIT_ACTIVE_CONTEXT_KEY,
|
||||
type ForeachActiveContext,
|
||||
@@ -5546,6 +5547,15 @@ export class TaskExecutor {
|
||||
*/
|
||||
private graphSeamThinkingLevel = new Map<string, ThinkingLevel>();
|
||||
|
||||
/**
|
||||
* FNXC:WorkflowStepSkills 2026-07-22-00:00:
|
||||
* FN-8490 pins the canonical `config.executor: "skill"` + trimmed
|
||||
* `config.skillName` request only for the pass-initiating foreach instance.
|
||||
* The implementation pass is shared across instances, so this template-constant
|
||||
* value must settle with the same lifecycle as governing-node and thinking pins.
|
||||
*/
|
||||
private graphSeamSkillName = new Map<string, string>();
|
||||
|
||||
/** Tasks currently being orchestrated by the graph runner. Process-wide for
|
||||
* the same reason as executingTaskLock (FN-4811): duplicate execute()
|
||||
* invocations can arrive from different TaskExecutor instances in one
|
||||
@@ -5994,6 +6004,7 @@ export class TaskExecutor {
|
||||
this.graphUnattendedRuns.delete(task.id);
|
||||
this.graphSeamGoverningNodeId.delete(task.id);
|
||||
this.graphSeamThinkingLevel.delete(task.id);
|
||||
this.graphSeamSkillName.delete(task.id);
|
||||
this.graphExecuteSelfRequeued.delete(task.id);
|
||||
// Per-instance keys: clear every instance slot owned by this task.
|
||||
const ctxPrefix = `${task.id}:`;
|
||||
@@ -6792,6 +6803,7 @@ export class TaskExecutor {
|
||||
instanceId?: string,
|
||||
governingNodeId?: string,
|
||||
thinkingLevel?: ThinkingLevel,
|
||||
skillName?: string,
|
||||
): Promise<{ success: boolean; error?: string }> {
|
||||
const active = this.foreachActiveForTask(task.id, instanceId);
|
||||
/*
|
||||
@@ -6826,6 +6838,9 @@ export class TaskExecutor {
|
||||
if (thinkingLevel) {
|
||||
this.graphSeamThinkingLevel.set(task.id, thinkingLevel);
|
||||
}
|
||||
if (skillName) {
|
||||
this.graphSeamSkillName.set(task.id, skillName);
|
||||
}
|
||||
phase = this.runImplementationPhase(task);
|
||||
this.graphStepRunOnce.set(task.id, phase);
|
||||
void phase
|
||||
@@ -6838,6 +6853,9 @@ export class TaskExecutor {
|
||||
if (thinkingLevel && this.graphSeamThinkingLevel.get(task.id) === thinkingLevel) {
|
||||
this.graphSeamThinkingLevel.delete(task.id);
|
||||
}
|
||||
if (skillName && this.graphSeamSkillName.get(task.id) === skillName) {
|
||||
this.graphSeamSkillName.delete(task.id);
|
||||
}
|
||||
});
|
||||
}
|
||||
try {
|
||||
@@ -6935,6 +6953,7 @@ export class TaskExecutor {
|
||||
active: ForeachActiveContext,
|
||||
governingNodeId?: string,
|
||||
thinkingLevel?: ThinkingLevel,
|
||||
skillName?: string,
|
||||
): Promise<RunTaskStepResult> {
|
||||
const worktreePath = active.worktreePath || live.worktree;
|
||||
const runStep = (idx: number) =>
|
||||
@@ -6944,6 +6963,7 @@ export class TaskExecutor {
|
||||
active.instanceId,
|
||||
governingNodeId,
|
||||
thinkingLevel,
|
||||
skillName,
|
||||
);
|
||||
|
||||
/*
|
||||
@@ -7078,12 +7098,15 @@ export class TaskExecutor {
|
||||
}
|
||||
this.graphStepActiveContext.set(this.graphActiveContextKey(task.id, active.instanceId), active);
|
||||
const stepGoverningNodeId = context[SEAM_GOVERNING_NODE_CONTEXT_KEY];
|
||||
const seamSkillName = context[SEAM_SKILL_NAME_CONTEXT_KEY];
|
||||
return await this.runProjectedGraphTaskStep(
|
||||
task,
|
||||
live,
|
||||
stepIndex,
|
||||
active,
|
||||
typeof stepGoverningNodeId === "string" ? stepGoverningNodeId : undefined,
|
||||
undefined,
|
||||
typeof seamSkillName === "string" && seamSkillName.trim() ? seamSkillName.trim() : undefined,
|
||||
);
|
||||
},
|
||||
resetTaskStep: async (ctx, task, stepIndex, baselineSha, checkpointId) => {
|
||||
@@ -7680,6 +7703,7 @@ export class TaskExecutor {
|
||||
// foreach (overwrite mid-build, or clear while the shared pass is live).
|
||||
const stepGoverningNodeId = context[SEAM_GOVERNING_NODE_CONTEXT_KEY];
|
||||
const seamThinkingLevel = context[SEAM_THINKING_LEVEL_CONTEXT_KEY];
|
||||
const seamSkillName = context[SEAM_SKILL_NAME_CONTEXT_KEY];
|
||||
const result = await this.runProjectedGraphTaskStep(
|
||||
seamTask,
|
||||
live,
|
||||
@@ -7689,6 +7713,7 @@ export class TaskExecutor {
|
||||
typeof seamThinkingLevel === "string" && WORKFLOW_THINKING_LEVEL_SET.has(seamThinkingLevel)
|
||||
? (seamThinkingLevel as ThinkingLevel)
|
||||
: undefined,
|
||||
typeof seamSkillName === "string" && seamSkillName.trim() ? seamSkillName.trim() : undefined,
|
||||
);
|
||||
// Capture baseline/checkpoint back into the reserved active context so the
|
||||
// foreach sub-walk threads them to later template nodes (step-review/reset).
|
||||
@@ -11619,6 +11644,39 @@ export class TaskExecutor {
|
||||
projectRootDir: this.rootDir,
|
||||
pluginRunner: this.options.pluginRunner,
|
||||
});
|
||||
const graphSeamSkillName = this.graphSeamSkillName.get(task.id);
|
||||
const ceSkillsDir = typeof taskEnv?.FUSION_CE_SKILLS_DIR === "string" && taskEnv.FUSION_CE_SKILLS_DIR.trim()
|
||||
? taskEnv.FUSION_CE_SKILLS_DIR.trim()
|
||||
: typeof process.env.FUSION_CE_SKILLS_DIR === "string" && process.env.FUSION_CE_SKILLS_DIR.trim()
|
||||
? process.env.FUSION_CE_SKILLS_DIR.trim()
|
||||
: undefined;
|
||||
let stepSessionSkillSelection = skillContext.skillSelectionContext;
|
||||
if (graphSeamSkillName) {
|
||||
const bare = graphSeamSkillName.includes(":")
|
||||
? graphSeamSkillName.slice(graphSeamSkillName.lastIndexOf(":") + 1)
|
||||
: graphSeamSkillName;
|
||||
const existing = stepSessionSkillSelection?.requestedSkillNames ?? [];
|
||||
stepSessionSkillSelection = {
|
||||
projectRootDir: stepSessionSkillSelection?.projectRootDir ?? this.rootDir,
|
||||
...(stepSessionSkillSelection?.sessionPurpose
|
||||
? { sessionPurpose: stepSessionSkillSelection.sessionPurpose }
|
||||
: { sessionPurpose: "executor" }),
|
||||
requestedSkillNames: [...new Set([...existing, graphSeamSkillName, bare])],
|
||||
};
|
||||
}
|
||||
const stepSessionAdditionalSkillPaths = mergeAdditionalSkillPaths(
|
||||
skillContext.additionalSkillPaths,
|
||||
graphSeamSkillName && ceSkillsDir ? [ceSkillsDir] : undefined,
|
||||
);
|
||||
if (
|
||||
graphSeamSkillName
|
||||
&& !isWorkflowStepSkillDiscoverable(graphSeamSkillName, stepSessionAdditionalSkillPaths, ceSkillsDir)
|
||||
) {
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`[skill-load] Foreach step-execute requests skill '${graphSeamSkillName}' but it cannot be discovered from configured plugin body directories or FUSION_CE_SKILLS_DIR; the step runs with role-fallback skills only.`,
|
||||
);
|
||||
}
|
||||
|
||||
// Graph-owned stepwise runs force step-session physics for the run (KTD-2/
|
||||
// KTD-8): the discrete per-step boundary the foreach driver needs exists only
|
||||
@@ -11678,8 +11736,8 @@ export class TaskExecutor {
|
||||
mcpServers: await this.resolveMcpServers(stepIdentityAgent?.id),
|
||||
workflowStepThinkingLevel: this.graphSeamThinkingLevel.get(task.id),
|
||||
// FNXC:PluginSkills 2026-07-12-00:00: Step sessions must forward plugin skill body dirs alongside requested names; otherwise plugin-provided SKILL.md bodies are invisible to the inner createFnAgent loader.
|
||||
skillSelection: skillContext.skillSelectionContext,
|
||||
additionalSkillPaths: skillContext.additionalSkillPaths,
|
||||
skillSelection: stepSessionSkillSelection,
|
||||
additionalSkillPaths: stepSessionAdditionalSkillPaths,
|
||||
// Pass agentStore and messageStore for delegation and messaging tools
|
||||
agentStore: this.options.agentStore,
|
||||
messageStore: this.options.messageStore,
|
||||
|
||||
@@ -55,6 +55,14 @@ export {
|
||||
// silent no-op).
|
||||
export const SEAM_THINKING_LEVEL_CONTEXT_KEY = "workflow:seamThinkingLevel";
|
||||
|
||||
/**
|
||||
* FNXC:WorkflowStepSkills 2026-07-22-00:00:
|
||||
* FN-8490 makes a foreach step-execute skill executor a first-class session
|
||||
* resource request. Only the canonical config-bag pair activates it, so a root
|
||||
* node field or a prose skill reference cannot accidentally pin a session skill.
|
||||
*/
|
||||
export const SEAM_SKILL_NAME_CONTEXT_KEY = "workflow:seamSkillName";
|
||||
|
||||
export type WorkflowSeamName =
|
||||
| "planning"
|
||||
| "execute"
|
||||
@@ -273,6 +281,17 @@ export function resolveSeamName(node: { config?: Record<string, unknown> }): Wor
|
||||
* Prompt/script handler: seam-configured nodes delegate to the legacy seam;
|
||||
* custom nodes run through the injected custom-node runner.
|
||||
*/
|
||||
function stampStepExecuteSkillName(node: WorkflowIrNode, context: Record<string, unknown>): void {
|
||||
const skillName = node.config?.executor === "skill" && typeof node.config.skillName === "string"
|
||||
? node.config.skillName.trim()
|
||||
: "";
|
||||
if (skillName) {
|
||||
context[SEAM_SKILL_NAME_CONTEXT_KEY] = skillName;
|
||||
} else {
|
||||
delete context[SEAM_SKILL_NAME_CONTEXT_KEY];
|
||||
}
|
||||
}
|
||||
|
||||
export function createPromptLikeHandler(
|
||||
seams: WorkflowLegacySeams,
|
||||
runCustomNode?: WorkflowCustomNodeRunner,
|
||||
@@ -316,6 +335,7 @@ export function createPromptLikeHandler(
|
||||
} else {
|
||||
delete context.context[SEAM_THINKING_LEVEL_CONTEXT_KEY];
|
||||
}
|
||||
stampStepExecuteSkillName(node, context.context);
|
||||
return seams.stepExecute(context.task, context.context);
|
||||
}
|
||||
if (seam) {
|
||||
@@ -357,6 +377,7 @@ export function createPrimitivePromptLikeHandler(
|
||||
active.stepIndex,
|
||||
node.id,
|
||||
);
|
||||
stampStepExecuteSkillName(node, context.context);
|
||||
const result = await primitives.runTaskStep(
|
||||
primitiveContextForNode(node, context.task, context.context, undefined, context.signal),
|
||||
context.task,
|
||||
|
||||
Reference in New Issue
Block a user