From 53e3063e9fdadc261bdd5b039a54dbe9353dcc4d Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 22 Jul 2026 15:07:59 -0700 Subject: [PATCH] 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) --- .changeset/fn-8490-step-execute-skill.md | 7 + docs/workflow-steps.md | 4 +- .../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(-) create mode 100644 .changeset/fn-8490-step-execute-skill.md create mode 100644 packages/engine/src/__tests__/step-execute-skill-loading.test.ts diff --git a/.changeset/fn-8490-step-execute-skill.md b/.changeset/fn-8490-step-execute-skill.md new file mode 100644 index 0000000000..88abb0b808 --- /dev/null +++ b/.changeset/fn-8490-step-execute-skill.md @@ -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). diff --git a/docs/workflow-steps.md b/docs/workflow-steps.md index d1c69c5011..d414d77bbf 100644 --- a/docs/workflow-steps.md +++ b/docs/workflow-steps.md @@ -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. diff --git a/packages/engine/src/__tests__/step-execute-skill-loading.test.ts b/packages/engine/src/__tests__/step-execute-skill-loading.test.ts new file mode 100644 index 0000000000..403bccbc16 --- /dev/null +++ b/packages/engine/src/__tests__/step-execute-skill-loading.test.ts @@ -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): 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 = { + [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 = { [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")); + }); +}); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 12b2448a95..2825bd745a 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -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(); + /** + * 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(); + /** 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 { 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, diff --git a/packages/engine/src/workflow-node-handlers.ts b/packages/engine/src/workflow-node-handlers.ts index 0df6b7e82a..df71ffd2bb 100644 --- a/packages/engine/src/workflow-node-handlers.ts +++ b/packages/engine/src/workflow-node-handlers.ts @@ -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 }): 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): 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,