Merge pull request #29 from HarryCordewener/fix/skill-discovery-path-doubling-and-matching

fix: skill discovery path doubling and pattern/name matching

.changeset/fix-skill-name-matching.md

@@ -0,0 +1,5 @@
+---
+"@runfusion/fusion": patch
+---
+
+Fix skill name matching between Fusion's two-segment names (e.g. `web-research/SKILL.md`) and pi-coding-agent's bare directory names (e.g. `web-research`). Patterns and requested skill names now strip the `/SKILL.md` suffix before comparison, eliminating spurious "not found in discovered skills" warnings.

packages/dashboard/src/skills-adapter.ts

@@ -261,8 +261,12 @@ export function createSkillsAdapter(options: {
       const discoveredSkills: DiscoveredSkill[] = [];
 
       for (const resource of skillResources) {
-        // Compute relative path for the skill
+        // Compute relative path for the skill.
-        const skillRelativePath = "skills/" + relative(resource.metadata.baseDir ?? "", resource.path);
+        // Guard against baseDir being the parent of the skills directory,
+        // which causes relative() to already include "skills/" in the result.
+        const relPath = relative(resource.metadata.baseDir ?? "", resource.path)
+          .replaceAll("\\", "/");
+        const skillRelativePath = relPath.startsWith("skills/") ? relPath : `skills/${relPath}`;
 
         const skillId = computeSkillId(resource.metadata.source, skillRelativePath);
         const skillName = extractSkillName(skillRelativePath, resource.metadata.source);

packages/engine/src/__tests__/skill-resolver.test.ts

@@ -961,5 +961,83 @@ describe("createSkillsOverrideFromSelection", () => {
       );
       expect(missingWarnings).toHaveLength(0);
     });
+
+    it("matches Fusion two-segment patterns against pi-coding-agent bare skill names", () => {
+      // Fusion's toggleExecutionSkill saves patterns like "+web-research/SKILL.md"
+      // and normalizeAgentSkills extracts "web-research/SKILL.md" from full IDs.
+      // But pi-coding-agent sets Skill.name to just the directory name: "web-research".
+      // This test verifies the cross-format matching works.
+      const dir = createMockProjectDir({
+        skills: ["+web-research/SKILL.md"],
+      });
+
+      const resolvedSkills = resolveSessionSkills({
+        projectRootDir: dir,
+        // Simulates what normalizeAgentSkills produces from "auto::skills/web-research/SKILL.md"
+        requestedSkillNames: ["web-research/SKILL.md"],
+        sessionPurpose: "triage",
+      });
+
+      const override = createSkillsOverrideFromSelection(resolvedSkills, {
+        requestedSkillNames: resolvedSkills.allowedSkillPaths.size > 0
+          ? ["web-research/SKILL.md"]
+          : undefined,
+        sessionPurpose: "triage",
+      });
+
+      // pi-coding-agent discovers skills with bare directory names
+      const base = {
+        skills: [
+          { name: "web-research", filePath: "/home/user/.pi/agent/skills/web-research/SKILL.md", description: "Web search", baseDir: "/home/user/.pi/agent/skills/web-research", sourceInfo: {} as any, disableModelInvocation: false },
+          { name: "paperclip", filePath: "/home/user/.pi/agent/skills/paperclip/SKILL.md", description: "Paperclip", baseDir: "/home/user/.pi/agent/skills/paperclip", sourceInfo: {} as any, disableModelInvocation: false },
+        ],
+        diagnostics: [],
+      };
+
+      const result = override(base);
+
+      // web-research should match despite the naming mismatch
+      expect(result.skills).toHaveLength(1);
+      expect(result.skills[0].name).toBe("web-research");
+
+      // No spurious "not found" warnings
+      const notFoundWarnings = result.diagnostics.filter(d =>
+        d.type === "warning" && d.message.includes("not found")
+      );
+      expect(notFoundWarnings).toHaveLength(0);
+    });
+
+    it("exclusion patterns with /SKILL.md suffix correctly exclude pi-discovered skills", () => {
+      const dir = createMockProjectDir({
+        skills: ["+web-research/SKILL.md", "-paperclip/SKILL.md"],
+      });
+
+      const resolvedSkills = resolveSessionSkills({
+        projectRootDir: dir,
+      });
+
+      const override = createSkillsOverrideFromSelection(resolvedSkills, {
+        sessionPurpose: "executor",
+      });
+
+      const base = {
+        skills: [
+          { name: "web-research", filePath: "/home/user/.pi/agent/skills/web-research/SKILL.md", description: "Web search", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
+          { name: "paperclip", filePath: "/home/user/.pi/agent/skills/paperclip/SKILL.md", description: "Paperclip", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
+        ],
+        diagnostics: [],
+      };
+
+      const result = override(base);
+
+      expect(result.skills).toHaveLength(1);
+      expect(result.skills[0].name).toBe("web-research");
+
+      // Should have a "disabled" diagnostic for paperclip (exists but excluded)
+      const disabledWarning = result.diagnostics.find(d =>
+        d.message.includes("disabled") && d.message.includes("paperclip")
+      );
+      expect(disabledWarning).toBeDefined();
+    });
   });
 });

packages/engine/src/session-skill-context.ts

@@ -101,10 +101,22 @@ export function normalizeAgentSkills(
       }
     }
 
-    // Skip invalid/empty entries and deduplicate
+    // Skip invalid/empty entries and deduplicate.
-    if (name && name.length > 0 && !seen.has(name)) {
+    // If the entry is a full skill ID ("source::path"), extract just the
-      seen.add(name);
+    // skill name (last two path segments) so it matches the discovered
-      result.push(name);
+    // skill.name produced by extractSkillName().
+    if (name && name.length > 0) {
+      if (name.includes("::")) {
+        const idPath = name.split("::").pop()!;
+        const parts = idPath.replace(/\\/g, "/").split("/").filter(Boolean);
+        if (parts.length >= 2) {
+          name = parts.slice(-2).join("/");
+        }
+      }
+      if (!seen.has(name)) {
+        seen.add(name);
+        result.push(name);
+      }
     }
   }
 

packages/engine/src/skill-resolver.ts

@@ -167,6 +167,24 @@ function isExclusionPattern(pattern: string): boolean {
   return pattern.startsWith("-");
 }
 
+/**
+ * Extract the bare skill name for matching purposes.
+ *
+ * Fusion conventions use two-segment names like "web-research/SKILL.md" (from
+ * extractSkillName and normalizeAgentSkills), but pi-coding-agent sets Skill.name
+ * to just the parent directory (e.g. "web-research"). This helper strips common
+ * suffixes so both sides can be compared:
+ *
+ *   "web-research/SKILL.md"           → "web-research"
+ *   "skills/web-research/SKILL.md"    → "web-research"
+ *   "web-research"                    → "web-research"
+ *   "/abs/path/skills/web-research/SKILL.md" → left unchanged (absolute paths
+ *     are matched by filePath comparison, not by this helper)
+ */
+function bareSkillName(name: string): string {
+  return name.replace(/\/SKILL\.md$/i, "");
+}
+
 // ── Main Resolution Logic ────────────────────────────────────────────────────
 
 /**
@@ -357,20 +375,45 @@ export function createSkillsOverrideFromSelection(
     // Skills must match the inclusion criteria AND not be in the exclusion list
     const hasExcluded = excludedSkillPaths.size > 0;
     let filteredSkills: Skill[];
+    // Build a name-based lookup for pattern/exclusion matching.
+    // Settings patterns are relative (e.g. "web-research/SKILL.md") but
+    // skill.filePath is absolute. Match against skill.name instead so
+    // that patterns written by toggleExecutionSkill() actually resolve.
+    //
+    // pi-coding-agent sets Skill.name to the parent directory name
+    // (e.g. "web-research") while Fusion uses two-segment names
+    // (e.g. "web-research/SKILL.md"). bareSkillName() normalizes
+    // both sides so the comparison succeeds.
+    const skillNameMatches = (skill: Skill, pattern: string): boolean =>
+      bareSkillName(skill.name).toLowerCase() === bareSkillName(pattern).toLowerCase()
+      || skill.filePath === pattern;
+    const isExcluded = (skill: Skill): boolean => {
+      for (const ep of excludedSkillPaths) {
+        if (skillNameMatches(skill, ep)) return true;
+      }
+      return false;
+    };
+    const isAllowed = (skill: Skill): boolean => {
+      for (const ap of allowedSkillPaths) {
+        if (skillNameMatches(skill, ap)) return true;
+      }
+      return false;
+    };
+
     if (hasRequestedNames) {
-      // Filter by requested names (case-insensitive match)
+      // Filter by requested names (case-insensitive match, normalize away /SKILL.md suffix)
-      const requestedNamesLower = new Set(requestedSkillNames!.map((n) => n.toLowerCase()));
+      const requestedBareNamesLower = new Set(requestedSkillNames!.map((n) => bareSkillName(n).toLowerCase()));
       filteredSkills = base.skills.filter(
-        (skill) => requestedNamesLower.has(skill.name.toLowerCase()) && !excludedSkillPaths.has(skill.filePath)
+        (skill) => requestedBareNamesLower.has(bareSkillName(skill.name).toLowerCase()) && !isExcluded(skill)
       );
     } else if (hasPatterns) {
-      // Filter by file path (in allowed set AND not in excluded set)
+      // Filter by pattern (allowed AND not excluded)
       filteredSkills = base.skills.filter(
-        (skill) => allowedSkillPaths.has(skill.filePath) && !excludedSkillPaths.has(skill.filePath)
+        (skill) => isAllowed(skill) && !isExcluded(skill)
       );
     } else if (hasExcluded) {
       // Only exclusions set - filter out excluded skills
-      filteredSkills = base.skills.filter((skill) => !excludedSkillPaths.has(skill.filePath));
+      filteredSkills = base.skills.filter((skill) => !isExcluded(skill));
     } else {
       // No filter criteria - this shouldn't happen if filterActive is true
       filteredSkills = base.skills;
@@ -380,28 +423,25 @@ export function createSkillsOverrideFromSelection(
     const newDiagnostics: ResourceDiagnostic[] = [];
 
     // Check for excluded paths that DO match a discovered skill (disabled)
-    // These are skills that exist but were explicitly excluded by project patterns
     const purpose = sessionPurpose ? ` [${sessionPurpose}]` : "";
-    const discoveredPaths = new Set(base.skills.map((s) => s.filePath));
+    const discoveredBareNames = new Set(base.skills.map((s) => bareSkillName(s.name).toLowerCase()));
+    const discoveredFilePaths = new Set(base.skills.map((s) => s.filePath));
+    const hasDiscoveredMatch = (pattern: string): boolean =>
+      discoveredBareNames.has(bareSkillName(pattern).toLowerCase()) || discoveredFilePaths.has(pattern);
 
     for (const excludedPath of excludedSkillPaths) {
-      if (discoveredPaths.has(excludedPath)) {
+      if (hasDiscoveredMatch(excludedPath)) {
-        // Skill exists but was disabled by project patterns
-        // Use "warning" type since ResourceDiagnostic only supports warning|error|collision
         newDiagnostics.push({
           type: "warning",
           message: `Skill at '${excludedPath}' exists but is disabled by project execution settings${purpose}`,
           path: excludedPath,
         });
       }
-      // If the path doesn't match any discovered skill, it's not a disabled skill - it's just not relevant
     }
 
     // Check for configured patterns (allowed paths) that don't match any discovered skill
-    // Note: At this point, we have access to base.skills for validation
     for (const allowedPath of allowedSkillPaths) {
-      if (!discoveredPaths.has(allowedPath)) {
+      if (!hasDiscoveredMatch(allowedPath)) {
-        // Allowed path doesn't match any discovered skill - this is a missing/invalid pattern
         newDiagnostics.push({
           type: "warning",
           message: `Configured skill pattern '${allowedPath}' not found in discovered skills${purpose}`,
@@ -412,10 +452,10 @@ export function createSkillsOverrideFromSelection(
 
     // Check for requested names that don't match any discovered skill
     if (requestedSkillNames) {
-      const discoveredNamesLower = new Set(base.skills.map((s) => s.name.toLowerCase()));
+      const discoveredBareNamesLower = new Set(base.skills.map((s) => bareSkillName(s.name).toLowerCase()));
       for (const requestedName of requestedSkillNames) {
         if (
-          !discoveredNamesLower.has(requestedName.toLowerCase())
+          !discoveredBareNamesLower.has(bareSkillName(requestedName).toLowerCase())
           && !isBuiltInFallbackRequest(requestedName)
         ) {
           const purpose = sessionPurpose ? ` [${sessionPurpose}]` : "";