From 648971634af7cc0621460e934840dd553dc80312 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Tue, 21 Jul 2026 19:05:48 -0700 Subject: [PATCH] FN-8461: suppress spurious workflow skill-load warnings Prevent optional CE configuration from producing warnings when a requested plugin skill is discoverable. - Merge plugin skill body directories with the optional CE discovery root - Warn only when the named workflow skill lacks every viable discovery source - Cover plugin, CE-namespaced, and unrelated-skill discovery cases Files changed: .changeset/fn-8461-skill-load-warning.md | 7 + docs/workflow-steps.md | 8 +- .../__tests__/ce-workflow-step-executor.test.ts | 149 ++++++++++++++++++++- .../engine/src/__tests__/executor-test-helpers.ts | 1 + packages/engine/src/executor.ts | 53 ++++++-- 5 files changed, 200 insertions(+), 18 deletions(-) Fusion-Task-Id: FN-8461 Fusion-Task-Lineage: ef743df4-8bd2-44e6-9498-f6448738d6dc Co-authored-by: Fusion (runfusion.ai) --- .changeset/fn-8461-skill-load-warning.md | 7 + docs/workflow-steps.md | 8 +- .../ce-workflow-step-executor.test.ts | 149 +++++++++++++++++- .../src/__tests__/executor-test-helpers.ts | 1 + packages/engine/src/executor.ts | 53 +++++-- 5 files changed, 200 insertions(+), 18 deletions(-) create mode 100644 .changeset/fn-8461-skill-load-warning.md diff --git a/.changeset/fn-8461-skill-load-warning.md b/.changeset/fn-8461-skill-load-warning.md new file mode 100644 index 0000000000..b234f48742 --- /dev/null +++ b/.changeset/fn-8461-skill-load-warning.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Stop false CE skill-load warnings when plugin skills resolve without FUSION_CE_SKILLS_DIR. +category: fix +dev: executeWorkflowStep warns [skill-load] only when the named skill is not discoverable after multi-source merge (plugin body dirs and/or FUSION_CE_SKILLS_DIR); unrelated plugin paths do not suppress a missing-name warning; successful non-CE plugin skill nodes no longer warn on unset FUSION_CE_SKILLS_DIR (GitHub #2388 / FN-8461). diff --git a/docs/workflow-steps.md b/docs/workflow-steps.md index e902e78cb9..d1c69c5011 100644 --- a/docs/workflow-steps.md +++ b/docs/workflow-steps.md @@ -88,11 +88,13 @@ Use this inventory as the documentation map for current workflow behavior: ### Skill-backed workflow steps -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`, then threads the injected `FUSION_CE_SKILLS_DIR` value as `additionalSkillPaths` so the bundled `SKILL.md` can be discovered. If a step names a skill but `FUSION_CE_SKILLS_DIR` is absent, the executor logs a `[skill-load]` warning instead of silently presenting only prompt text while falling back to role skills. +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. + +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. ### Custom workflow authoring diff --git a/packages/engine/src/__tests__/ce-workflow-step-executor.test.ts b/packages/engine/src/__tests__/ce-workflow-step-executor.test.ts index 34bfb07671..16d52c10a8 100644 --- a/packages/engine/src/__tests__/ce-workflow-step-executor.test.ts +++ b/packages/engine/src/__tests__/ce-workflow-step-executor.test.ts @@ -23,10 +23,14 @@ * line on prompt so the parse path completes cleanly. */ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { BUILTIN_WORKFLOWS, type WorkflowIr } from "@fusion/core"; import "./executor-test-helpers.js"; import { TaskExecutor } from "../executor.js"; +import type { PluginRunner } from "../plugin-runner.js"; import { WorkflowGraphExecutor } from "../workflow-graph-executor.js"; import { createMockStore, @@ -97,12 +101,52 @@ function captureSession( return holder; } -function makeExecutor(store: ReturnType) { +function makeExecutor( + store: ReturnType, + pluginRunner?: PluginRunner, +) { const agentStore = { getAgent: vi.fn().mockResolvedValue(null), createAgent: vi.fn() }; - const executor = new TaskExecutor(store as any, "/tmp/test", { agentStore } as any); + const executor = new TaskExecutor(store as any, "/tmp/test", { agentStore, pluginRunner } as any); return { executor, agentStore }; } +const tempDirs: string[] = []; + +async function createPluginSkillFixture(skillName: string, body: string): Promise<{ pluginRoot: string; skillDir: string; skillFile: string }> { + const pluginRoot = await mkdtemp(join(tmpdir(), "workflow-step-plugin-skill-")); + tempDirs.push(pluginRoot); + const skillDir = join(pluginRoot, "skills", skillName); + const skillFile = join(skillDir, "SKILL.md"); + await mkdir(skillDir, { recursive: true }); + await writeFile(skillFile, `---\nname: ${skillName}\ndescription: Test plugin skill\n---\n\n${body}`, "utf-8"); + return { pluginRoot, skillDir, skillFile }; +} + +async function expectCapturedSkillBody( + cap: ReturnType, + projectRootDir: string, + agentDir: string, + skillName: string, + skillFile: string, + distinctiveBody: string, +) { + const { DefaultResourceLoader } = await vi.importActual("@earendil-works/pi-coding-agent"); + const { createSkillsOverrideFromSelection, resolveSessionSkills } = await vi.importActual("../skill-resolver.js"); + const requestedSkillNames = cap.last?.skillSelection?.requestedSkillNames; + const selection = resolveSessionSkills({ projectRootDir, requestedSkillNames, sessionPurpose: "executor" }); + const loader = new DefaultResourceLoader({ + cwd: projectRootDir, + agentDir, + additionalSkillPaths: cap.last?.additionalSkillPaths, + skillsOverride: createSkillsOverrideFromSelection(selection, { requestedSkillNames, sessionPurpose: "executor" }), + }); + await loader.reload(); + + const skill = (loader.getSkills().skills as Array<{ name: string; filePath: string }>).find((candidate) => candidate.name === skillName); + expect(skill?.filePath).toBe(skillFile); + await expect(readFile(skillFile, "utf-8")).resolves.toContain(distinctiveBody); +} + function baseStepTask(overrides: Record = {}) { return { id: "FN-CE-1", @@ -186,8 +230,13 @@ function quietGit() { } describe("CE workflow-step executor integration", () => { + afterEach(async () => { + await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true }))); + }); + beforeEach(() => { resetExecutorMocks(); + mockedExistsSync.mockReturnValue(true); quietGit(); }); @@ -1170,7 +1219,7 @@ Ship FIVE kinds. Do NOT add roadmap-item in this task. }, ); - it("warns loudly and does not set additionalSkillPaths when a skill step lacks FUSION_CE_SKILLS_DIR", async () => { + it("warns when the named CE skill has no viable multi-source discovery path", async () => { const store = createMockStore(); const { executor } = makeExecutor(store); const cap = captureSession(); @@ -1189,10 +1238,100 @@ Ship FIVE kinds. Do NOT add roadmap-item in this task. expect(requested).toContain("ce-work"); expect(cap.last?.additionalSkillPaths).toBeUndefined(); expect(skillLoadWarnings(store)).toEqual([ - "[skill-load] Workflow step 'Execute' requests skill 'compound-engineering:ce-work' but FUSION_CE_SKILLS_DIR is unset — the skill cannot be discovered; the step runs with role-fallback skills only.", + "[skill-load] Workflow step 'Execute' requests skill 'compound-engineering:ce-work' but it cannot be discovered from configured plugin body directories or FUSION_CE_SKILLS_DIR; the step runs with role-fallback skills only.", ]); }); + it("does not warn when the requested plugin skill body is discoverable without a CE directory", async () => { + const distinctiveBody = "Security scan methodology from the plugin fixture."; + const { pluginRoot, skillDir, skillFile } = await createPluginSkillFixture("security-scan", distinctiveBody); + const projectRootDir = await mkdtemp(join(tmpdir(), "workflow-step-project-")); + const agentDir = await mkdtemp(join(tmpdir(), "workflow-step-agent-")); + tempDirs.push(projectRootDir, agentDir); + await mkdir(join(projectRootDir, ".fusion"), { recursive: true }); + mockedExistsSync.mockImplementation((path) => String(path) === skillFile); + + const pluginRunner = { + getPluginSkills: vi.fn().mockReturnValue([ + { pluginId: "plugin-security", pluginRoot, skill: { name: "security-scan" } }, + ]), + } as unknown as PluginRunner; + const store = createMockStore(); + const { executor } = makeExecutor(store, pluginRunner); + const cap = captureSession(); + + await (executor as any).executeWorkflowStep( + baseStepTask(), + makeStep({ name: "Security scan", skillName: "security-scan" }), + "/tmp/wt", + {}, + {}, + undefined, + ); + + expect(skillLoadWarnings(store)).toEqual([]); + expect(cap.last?.skillSelection?.requestedSkillNames).toContain("security-scan"); + expect(cap.last?.additionalSkillPaths).toEqual([skillDir, dirname(skillDir)]); + await expectCapturedSkillBody(cap, projectRootDir, agentDir, "security-scan", skillFile, distinctiveBody); + }); + + it("still warns when only an unrelated plugin skill body is discoverable", async () => { + const { pluginRoot, skillDir, skillFile } = await createPluginSkillFixture("other-skill", "Unrelated plugin skill."); + mockedExistsSync.mockImplementation((path) => String(path) === skillFile); + const pluginRunner = { + getPluginSkills: vi.fn().mockReturnValue([ + { pluginId: "plugin-other", pluginRoot, skill: { name: "other-skill" } }, + ]), + } as unknown as PluginRunner; + const store = createMockStore(); + const { executor } = makeExecutor(store, pluginRunner); + const cap = captureSession(); + + await (executor as any).executeWorkflowStep( + baseStepTask(), + makeStep({ name: "Security scan", skillName: "security-scan" }), + "/tmp/wt", + {}, + {}, + undefined, + ); + + expect(cap.last?.additionalSkillPaths).toEqual([skillDir, dirname(skillDir)]); + expect(skillLoadWarnings(store)).toHaveLength(1); + }); + + it("does not warn when a CE-namespaced skill has a plugin-delivered body", async () => { + const distinctiveBody = "CE work methodology delivered from a plugin fixture."; + const { pluginRoot, skillDir, skillFile } = await createPluginSkillFixture("ce-work", distinctiveBody); + const projectRootDir = await mkdtemp(join(tmpdir(), "workflow-step-project-")); + const agentDir = await mkdtemp(join(tmpdir(), "workflow-step-agent-")); + tempDirs.push(projectRootDir, agentDir); + await mkdir(join(projectRootDir, ".fusion"), { recursive: true }); + mockedExistsSync.mockImplementation((path) => String(path) === skillFile); + const pluginRunner = { + getPluginSkills: vi.fn().mockReturnValue([ + { pluginId: "plugin-ce", pluginRoot, skill: { name: "ce-work" } }, + ]), + } as unknown as PluginRunner; + const store = createMockStore(); + const { executor } = makeExecutor(store, pluginRunner); + const cap = captureSession(); + + await (executor as any).executeWorkflowStep( + baseStepTask(), + makeStep({ name: "CE work", skillName: "compound-engineering:ce-work" }), + "/tmp/wt", + {}, + {}, + undefined, + ); + + expect(skillLoadWarnings(store)).toEqual([]); + expect(cap.last?.skillSelection?.requestedSkillNames).toEqual(expect.arrayContaining(["compound-engineering:ce-work", "ce-work"])); + expect(cap.last?.additionalSkillPaths).toEqual([skillDir, dirname(skillDir)]); + await expectCapturedSkillBody(cap, projectRootDir, agentDir, "ce-work", skillFile, distinctiveBody); + }); + it("a skill-less step contributes no skillName merge, no additionalSkillPaths, and no skill-load warning", async () => { const store = createMockStore(); const { executor } = makeExecutor(store); diff --git a/packages/engine/src/__tests__/executor-test-helpers.ts b/packages/engine/src/__tests__/executor-test-helpers.ts index 3683fc997c..9c47223f75 100644 --- a/packages/engine/src/__tests__/executor-test-helpers.ts +++ b/packages/engine/src/__tests__/executor-test-helpers.ts @@ -55,6 +55,7 @@ vi.mock("../logger.js", () => { ipcLog: createMockLogger(), projectManagerLog: createMockLogger(), hybridExecutorLog: createMockLogger(), + piLog: createMockLogger(), formatError: (err: unknown) => { if (err instanceof Error) { const message = err.message || err.name || "Error"; diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index be9409c886..430416c976 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -9,7 +9,7 @@ const execFileAsync = promisify(execFile); const WORKFLOW_THINKING_LEVEL_SET: ReadonlySet = new Set(THINKING_LEVELS); -import { delimiter, isAbsolute, join, relative, resolve as resolvePath } from "node:path"; +import { basename, delimiter, isAbsolute, join, relative, resolve as resolvePath } from "node:path"; import { existsSync, lstatSync, realpathSync } from "node:fs"; import { readFile, rm, writeFile } from "node:fs/promises"; import type { TaskStore, Task, TaskDetail, TaskTokenUsage, StepStatus, Settings, WorkflowStep, MissionStore, AsyncMissionStore, Slice, AgentState, AgentCapability, RunMutationContext, AgentHeartbeatConfig, Agent, AgentMemoryInclusionMode, ProjectSettings, MergeResult, WorkflowIrNode, WorkflowIrNodeKind, WorkflowStepResult as CoreWorkflowStepResult, ThinkingLevel } from "@fusion/core"; @@ -322,6 +322,36 @@ function mergeAdditionalSkillPaths(...pathGroups: Array): return merged.length > 0 ? merged : undefined; } +/** + * FNXC:WorkflowSteps 2026-08-08-00:00: + * FN-8461 / GitHub #2388 require workflow skill-load warnings to describe a true + * named-skill delivery failure, not an optional Compound Engineering source being + * absent. Plugin body directories are paired with their parent discovery roots, + * so check the requested bare name against each merged source; unrelated paths + * must never hide a missing requested skill. + */ +function isWorkflowStepSkillDiscoverable( + skillName: string, + additionalSkillPaths: string[] | undefined, + ceSkillsDir: string | undefined, +): boolean { + // A configured CE root remains a viable source by contract: deployments can + // inject a synthetic install root before its skill tree is materialized locally. + if (ceSkillsDir) return true; + + const bareSkillName = skillName.includes(":") + ? skillName.slice(skillName.lastIndexOf(":") + 1) + : skillName; + if (!bareSkillName || basename(bareSkillName) !== bareSkillName || bareSkillName === "." || bareSkillName === "..") { + return false; + } + + return (additionalSkillPaths ?? []).some((skillPath) => + (basename(skillPath) === bareSkillName && existsSync(join(skillPath, "SKILL.md"))) + || existsSync(join(skillPath, bareSkillName, "SKILL.md")), + ); +} + const yieldEventLoop = (): Promise => new Promise((resolve) => setImmediateCb(resolve)); function getPromptSection(prompt: string, heading: string): string { @@ -16811,19 +16841,22 @@ You have access to the file system to review changes.${inlineFixBlock}${verdictB requestedSkillNames: mergedNames, }; } - // FNXC:WorkflowSteps 2026-06-20-23:35: - // A named skill with no discovery path silently degrades to the role-fallback - // skill (the exact pre-fix bug this change exists to kill). If the injected - // FUSION_CE_SKILLS_DIR never arrived (degraded/throwing plugin, missing install - // dir), warn loudly so an env-threading regression is visible on a board run - // instead of failing silent with a green hand-fed test. - if (workflowStep.skillName && workflowStep.skillName.trim() && !ceSkillsDir) { + const additionalSkillPaths = mergeAdditionalSkillPaths(skillContext.additionalSkillPaths, ceSkillsDir ? [ceSkillsDir] : undefined); + // FNXC:WorkflowSteps 2026-08-08-00:00: + // FN-8461 / GitHub #2388: workflow steps resolve skills from enabled-plugin + // body directories and the optional CE install root. Warn only after merging + // those sources when THIS named skill remains undiscoverable: a non-empty path + // array for another skill is not viable, while an actual plugin body makes CE + // env absence expected rather than misleading operator-facing noise. + if ( + workflowStep.skillName?.trim() + && !isWorkflowStepSkillDiscoverable(workflowStep.skillName.trim(), additionalSkillPaths, ceSkillsDir) + ) { await this.store.logEntry( task.id, - `[skill-load] Workflow step '${workflowStep.name}' requests skill '${workflowStep.skillName}' but FUSION_CE_SKILLS_DIR is unset — the skill cannot be discovered; the step runs with role-fallback skills only.`, + `[skill-load] Workflow step '${workflowStep.name}' requests skill '${workflowStep.skillName}' but it cannot be discovered from configured plugin body directories or FUSION_CE_SKILLS_DIR; the step runs with role-fallback skills only.`, ); } - const additionalSkillPaths = mergeAdditionalSkillPaths(skillContext.additionalSkillPaths, ceSkillsDir ? [ceSkillsDir] : undefined); const logBrowserVerificationActivity = async (message: string) => { await this.store.logEntry(task.id, message); await this.store.appendAgentLog(task.id, message, "status", undefined, "reviewer");