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) <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-8461-skill-load-warning.md
Normal file
7
.changeset/fn-8461-skill-load-warning.md
Normal file
@@ -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).
|
||||
@@ -88,11 +88,13 @@ Use this inventory as the documentation map for current workflow behavior:
|
||||
### Skill-backed workflow steps
|
||||
|
||||
<!--
|
||||
FNXC:WorkflowSteps 2026-06-27-17:05:
|
||||
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. The engine passes both namespaced and bare request forms plus the injected Compound Engineering discovery path so bundled `SKILL.md` files are discoverable, and logs a loud warning when that path is missing.
|
||||
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`, 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
|
||||
|
||||
|
||||
@@ -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<typeof createMockStore>) {
|
||||
function makeExecutor(
|
||||
store: ReturnType<typeof createMockStore>,
|
||||
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<typeof captureSession>,
|
||||
projectRootDir: string,
|
||||
agentDir: string,
|
||||
skillName: string,
|
||||
skillFile: string,
|
||||
distinctiveBody: string,
|
||||
) {
|
||||
const { DefaultResourceLoader } = await vi.importActual<typeof import("@earendil-works/pi-coding-agent")>("@earendil-works/pi-coding-agent");
|
||||
const { createSkillsOverrideFromSelection, resolveSessionSkills } = await vi.importActual<typeof import("../skill-resolver.js")>("../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<string, unknown> = {}) {
|
||||
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);
|
||||
|
||||
@@ -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";
|
||||
|
||||
@@ -9,7 +9,7 @@ const execFileAsync = promisify(execFile);
|
||||
|
||||
const WORKFLOW_THINKING_LEVEL_SET: ReadonlySet<string> = 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<string[] | undefined>):
|
||||
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<void> => 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");
|
||||
|
||||
Reference in New Issue
Block a user