FN-8465: align skill toggles with canonical paths
Align session skill filtering with Skills-view path identities. - Match session inclusions, exclusions, and diagnostics by skills-relative body path. - Ignore stale flat toggle keys for categorized skills and cover the display/session invariant. - Document the behavior and add a patch changeset. Files changed: .../fn-8465-legacy-skill-toggle-path-match.md | 7 ++ docs/dashboard-guide.md | 2 +- .../legacy-flat-skill-toggle-session-divergence.md | 41 ++++++++ .../engine/src/__tests__/skill-resolver.test.ts | 73 ++++++++++++++ packages/engine/src/skill-resolver.ts | 105 ++++++++++++++------- 5 files changed, 191 insertions(+), 37 deletions(-) Fusion-Task-Id: FN-8465 Fusion-Task-Lineage: 042fc7f4-08a3-42f4-9a6f-52f066d585e8 Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-8465-legacy-skill-toggle-path-match.md
Normal file
7
.changeset/fn-8465-legacy-skill-toggle-path-match.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Ignore stale flat skill-toggle keys so session skills match the Skills view after category layouts.
|
||||
category: fix
|
||||
dev: Session skillsOverride matches +/- patterns by skills/-relative path (not bareSkillName alone); legacy flat disables no longer suppress nested skillFiles bodies (GitHub #2385 / FN-8465).
|
||||
@@ -1620,7 +1620,7 @@ For setup prerequisites, security caveats for tokenized URLs/QR links, and troub
|
||||
|
||||
The Skills view now supports the full browse-and-install loop for skills.sh entries: use **Skills Catalog** to search the catalog, click **Install** on any card with a source repository, and the dashboard will run the same installer as the CLI (`npx skills add <owner/repo> -y -a pi`, with `--skill <slug>` when applicable). On success, the view refreshes **Discovered Skills** immediately so the newly installed skill appears without a page reload.
|
||||
|
||||
The Skills API provides endpoints for managing execution skills. Skills are toggled via project-scoped settings in `.fusion/settings.json`.
|
||||
The Skills API provides endpoints for managing execution skills. Skills are toggled via project-scoped settings in `.fusion/settings.json`. Toggle entries match the skill body’s relative path beneath `skills/` (for example, `api/api-versioning/SKILL.md`), not just its displayed name. Stale flat-layout entries such as `-api-versioning/SKILL.md` are ignored for skills that now use a categorized body path, keeping the Skills view and agent-session manifest aligned.
|
||||
|
||||

|
||||
|
||||
|
||||
@@ -0,0 +1,41 @@
|
||||
---
|
||||
title: "Legacy flat skill-toggle keys silently suppressed categorized session skills"
|
||||
date: 2026-07-21
|
||||
category: docs/solutions/logic-errors
|
||||
module: engine (session skill resolver)
|
||||
problem_type: logic_error
|
||||
component: path_identity
|
||||
symptoms:
|
||||
- "The Skills view showed a categorized plugin skill enabled while agent sessions omitted it."
|
||||
- "Deleting a stale flat `-name/SKILL.md` settings entry restored the skill to the session manifest."
|
||||
root_cause: logic_error
|
||||
resolution_type: code_fix
|
||||
severity: medium
|
||||
related_components:
|
||||
- skill-resolver
|
||||
- skill-settings
|
||||
- skills-adapter
|
||||
tags:
|
||||
- skills
|
||||
- settings
|
||||
- plugin-skills
|
||||
- path-identity
|
||||
- session-manifest
|
||||
- legacy-settings
|
||||
---
|
||||
|
||||
# Legacy flat skill-toggle keys silently suppressed categorized session skills
|
||||
|
||||
## Problem
|
||||
|
||||
Project skill toggles are persisted as signed paths such as `-api/api-versioning/SKILL.md`. The Skills view resolves those paths relative to a skill body beneath `skills/`, but the session override resolver also accepted a bare skill-name match. After a plugin moved a body from `skills/api-versioning/SKILL.md` to `skills/api/api-versioning/SKILL.md`, an old `-api-versioning/SKILL.md` entry no longer matched the view yet still excluded the session skill because pi exposed `skill.name === "api-versioning"`.
|
||||
|
||||
## Solution
|
||||
|
||||
Session allow, exclusion, and disabled-diagnostic matching now use the same canonical identity as the Skills view: the complete body-relative path beneath the final `skills/` segment. Absolute patterns continue to match exact file paths. A flat `name/SKILL.md` pattern remains compatible only when the discovered body path is itself flat; it cannot match a categorized `category/name/SKILL.md` body merely by sharing the final directory name.
|
||||
|
||||
## Prevention
|
||||
|
||||
- Treat stored setting keys as identities, not labels: compare the complete normalized path at every reader.
|
||||
- When a layout changes, test every reader of the persisted key, including session assembly and diagnostics, not only the UI display.
|
||||
- Preserve legacy compatibility only where the legacy key still identifies the current on-disk path; otherwise ignore it consistently rather than allowing one reader to honor it.
|
||||
@@ -2,6 +2,7 @@
|
||||
* Unit tests for skill resolver.
|
||||
*/
|
||||
|
||||
import { resolvePluginSkillEnabled } from "@fusion/core";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
/*
|
||||
@@ -1152,6 +1153,78 @@ describe("createSkillsOverrideFromSelection", () => {
|
||||
expect(notFoundInfoDiagnostics).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("keeps nested skills available when legacy flat exclusions are ignored by the Skills view", () => {
|
||||
const settings = { skills: ["-api-versioning/SKILL.md"] };
|
||||
const dir = createMockProjectDir(settings);
|
||||
const selection = resolveSessionSkills({ projectRootDir: dir });
|
||||
const result = createSkillsOverrideFromSelection(selection, {
|
||||
sessionPurpose: "executor",
|
||||
})({
|
||||
skills: [
|
||||
{ name: "api-versioning", filePath: "/plugins/foo/skills/api/api-versioning/SKILL.md", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
{ name: "unrelated", filePath: "/plugins/foo/skills/other/unrelated/SKILL.md", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
],
|
||||
diagnostics: [],
|
||||
});
|
||||
|
||||
expect(resolvePluginSkillEnabled(
|
||||
settings,
|
||||
"foo",
|
||||
"api-versioning",
|
||||
true,
|
||||
"skills/api/api-versioning/SKILL.md",
|
||||
)).toBe(true);
|
||||
expect(result.skills.map((skill) => skill.name)).toEqual(["api-versioning", "unrelated"]);
|
||||
expect(result.diagnostics.some((diagnostic) =>
|
||||
diagnostic.message.includes("api-versioning/SKILL.md") && diagnostic.message.includes("disabled"),
|
||||
)).toBe(false);
|
||||
});
|
||||
|
||||
it("does not let a legacy flat enable create an empty allow-list for nested skills", () => {
|
||||
const settings = { skills: ["+api-versioning/SKILL.md"] };
|
||||
const dir = createMockProjectDir(settings);
|
||||
const selection = resolveSessionSkills({ projectRootDir: dir });
|
||||
const result = createSkillsOverrideFromSelection(selection)({
|
||||
skills: [
|
||||
{ name: "api-versioning", filePath: "/plugins/foo/skills/api/api-versioning/SKILL.md", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
{ name: "unrelated", filePath: "/plugins/foo/skills/other/unrelated/SKILL.md", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
],
|
||||
diagnostics: [],
|
||||
});
|
||||
|
||||
expect(resolvePluginSkillEnabled(
|
||||
settings,
|
||||
"foo",
|
||||
"api-versioning",
|
||||
true,
|
||||
"skills/api/api-versioning/SKILL.md",
|
||||
)).toBe(true);
|
||||
expect(result.skills.map((skill) => skill.name)).toEqual(["api-versioning", "unrelated"]);
|
||||
expect(result.diagnostics.some((diagnostic) =>
|
||||
diagnostic.message.includes("api-versioning/SKILL.md") && diagnostic.message.includes("not found"),
|
||||
)).toBe(false);
|
||||
});
|
||||
|
||||
it.each([
|
||||
"-api/api-versioning/SKILL.md",
|
||||
"-skills/api/api-versioning/SKILL.md",
|
||||
])("excludes nested skills for the canonical path %s", (pattern) => {
|
||||
const dir = createMockProjectDir({ skills: [pattern] });
|
||||
const selection = resolveSessionSkills({ projectRootDir: dir });
|
||||
const result = createSkillsOverrideFromSelection(selection)({
|
||||
skills: [
|
||||
{ name: "api-versioning", filePath: "/plugins/foo/skills/api/api-versioning/SKILL.md", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
],
|
||||
diagnostics: [],
|
||||
});
|
||||
|
||||
expect(result.skills).toEqual([]);
|
||||
expect(result.diagnostics).toContainEqual(expect.objectContaining({
|
||||
type: "warning",
|
||||
message: expect.stringContaining("disabled by project execution settings"),
|
||||
}));
|
||||
});
|
||||
|
||||
it("exclusion patterns with /SKILL.md suffix correctly exclude pi-discovered skills", () => {
|
||||
const dir = createMockProjectDir({
|
||||
skills: ["+web-research/SKILL.md", "-paperclip/SKILL.md"],
|
||||
|
||||
@@ -12,7 +12,7 @@
|
||||
import { existsSync, readFileSync } from "node:fs";
|
||||
import { dirname, join, resolve } from "node:path";
|
||||
import type { ResourceDiagnostic, Skill } from "@earendil-works/pi-coding-agent";
|
||||
import { getProjectRootFromWorktree } from "@fusion/core";
|
||||
import { getProjectRootFromWorktree, normalizeStoredSkillPath } from "@fusion/core";
|
||||
import { piLog } from "./logger.js";
|
||||
|
||||
// ── Project Root Resolution ──────────────────────────────────────────────────
|
||||
@@ -173,21 +173,59 @@ function isExclusionPattern(pattern: string): boolean {
|
||||
}
|
||||
|
||||
/**
|
||||
* 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)
|
||||
* Return the canonical body path beneath a discovered skill's `skills/` root.
|
||||
* A path without that root is only eligible for exact filePath matching.
|
||||
*/
|
||||
function bareSkillName(name: string): string {
|
||||
return name.replace(/\/SKILL\.md$/i, "");
|
||||
function skillBodyRelativePath(filePath: string): string | undefined {
|
||||
const normalizedFilePath = filePath.replaceAll("\\", "/");
|
||||
const lowerCasePath = normalizedFilePath.toLowerCase();
|
||||
const segmentIndex = lowerCasePath.lastIndexOf("/skills/");
|
||||
|
||||
if (segmentIndex >= 0) {
|
||||
return normalizedFilePath.slice(segmentIndex + "/skills/".length);
|
||||
}
|
||||
if (lowerCasePath.startsWith("skills/")) {
|
||||
return normalizedFilePath.slice("skills/".length);
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* FNXC:SkillResolution 2026-07-21-00:00:
|
||||
* GitHub #2385 / FN-8465 requires session filtering to use the same skills-relative body-path identity as the Skills view. Legacy `-name/SKILL.md` entries must not suppress a re-categorized `skills/category/name/SKILL.md` body merely because pi exposes the same bare Skill.name.
|
||||
*/
|
||||
function skillMatchesExecutionPattern(skill: Skill, pattern: string): boolean {
|
||||
if (skill.filePath === pattern) {
|
||||
return true;
|
||||
}
|
||||
|
||||
const bodyRelativePath = skillBodyRelativePath(skill.filePath);
|
||||
return bodyRelativePath !== undefined
|
||||
&& normalizeStoredSkillPath(bodyRelativePath).toLowerCase()
|
||||
=== normalizeStoredSkillPath(pattern).toLowerCase();
|
||||
}
|
||||
|
||||
/**
|
||||
* FNXC:SkillResolution 2026-07-21-00:00:
|
||||
* Legacy flat `+name/SKILL.md` keys must be ignored as well as legacy disables.
|
||||
* Otherwise an unmatched stale allow key activates the session allow-list and
|
||||
* suppresses re-categorized skills even though the Skills view shows them enabled.
|
||||
*/
|
||||
function isLegacyFlatPatternForNestedSkill(skill: Skill, pattern: string): boolean {
|
||||
const bodyRelativePath = skillBodyRelativePath(skill.filePath);
|
||||
if (!bodyRelativePath) return false;
|
||||
|
||||
const bodySegments = normalizeStoredSkillPath(bodyRelativePath)
|
||||
.toLowerCase()
|
||||
.split("/");
|
||||
const patternSegments = normalizeStoredSkillPath(pattern)
|
||||
.toLowerCase()
|
||||
.split("/");
|
||||
|
||||
return bodySegments.length > 2
|
||||
&& patternSegments.length === 2
|
||||
&& bodySegments.at(-2) === patternSegments[0]
|
||||
&& bodySegments.at(-1) === patternSegments[1];
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -398,34 +436,31 @@ export function createSkillsOverrideFromSelection(
|
||||
// Determine the effective filter criteria
|
||||
// When requestedSkillNames is provided without patterns, filter by name
|
||||
// When patterns are provided, filter by file path
|
||||
const hasPatterns = allowedSkillPaths.size > 0;
|
||||
const hasRequestedNames = Boolean(requestedSkillNames && requestedSkillNames.length > 0);
|
||||
|
||||
// A stale flat enable must not turn into an empty allow-list after a body
|
||||
// moved to a category. Keep truly missing configured patterns intact so
|
||||
// their existing missing-pattern diagnostic and filtering semantics remain.
|
||||
const effectiveAllowedSkillPaths = new Set(
|
||||
[...allowedSkillPaths].filter((pattern) => !base.skills.some(
|
||||
(skill) => isLegacyFlatPatternForNestedSkill(skill, pattern),
|
||||
)),
|
||||
);
|
||||
const hasPatterns = effectiveAllowedSkillPaths.size > 0;
|
||||
|
||||
// Filter skills
|
||||
// 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;
|
||||
for (const excludedPath of excludedSkillPaths) {
|
||||
if (skillMatchesExecutionPattern(skill, excludedPath)) return true;
|
||||
}
|
||||
return false;
|
||||
};
|
||||
const isAllowed = (skill: Skill): boolean => {
|
||||
for (const ap of allowedSkillPaths) {
|
||||
if (skillNameMatches(skill, ap)) return true;
|
||||
for (const allowedPath of effectiveAllowedSkillPaths) {
|
||||
if (skillMatchesExecutionPattern(skill, allowedPath)) return true;
|
||||
}
|
||||
return false;
|
||||
};
|
||||
@@ -454,10 +489,8 @@ export function createSkillsOverrideFromSelection(
|
||||
|
||||
// Check for excluded paths that DO match a discovered skill (disabled)
|
||||
const purpose = sessionPurpose ? ` [${sessionPurpose}]` : "";
|
||||
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);
|
||||
base.skills.some((skill) => skillMatchesExecutionPattern(skill, pattern));
|
||||
|
||||
for (const excludedPath of excludedSkillPaths) {
|
||||
if (hasDiscoveredMatch(excludedPath)) {
|
||||
@@ -470,7 +503,7 @@ export function createSkillsOverrideFromSelection(
|
||||
}
|
||||
|
||||
// Check for configured patterns (allowed paths) that don't match any discovered skill
|
||||
for (const allowedPath of allowedSkillPaths) {
|
||||
for (const allowedPath of effectiveAllowedSkillPaths) {
|
||||
if (!hasDiscoveredMatch(allowedPath)) {
|
||||
newDiagnostics.push({
|
||||
type: "info" as ResourceDiagnostic["type"],
|
||||
|
||||
Reference in New Issue
Block a user