FN-7777: fix skill content/file routes double-decoding skill IDs
Fixes Skills view showing "Skill not found" for every skill, including built-in Fusion skills, by removing a redundant URL decode in the content/file lookup routes. - register-agent-skills-routes.ts: stop calling decodeURIComponent on req.params.id in the /skills/:id/content and /skills/:id/file handlers since Express 5 already decodes route params once; a second decode corrupted encoded source segments so IDs no longer matched computeSkillId's discovery output - Added FNXC:Skills comments documenting the Express 5 single-decode behavior and why re-decoding breaks plugin/npm/path source IDs - routes-skills.test.ts: added regression coverage for exact-ID content/file lookups across skill sources - Added a patch changeset describing the fix for release notes Files changed: .changeset/fn-7777-skills-content-not-found.md | 7 + .../dashboard/src/__tests__/routes-skills.test.ts | 144 ++++++++++++++++++++- .../src/routes/register-agent-skills-routes.ts | 26 ++-- 3 files changed, 159 insertions(+), 18 deletions(-) Fusion-Task-Id: FN-7777 Fusion-Task-Lineage: dc190b9a-ac5c-4f8b-9d65-11bfd5598db2 Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-7777-skills-content-not-found.md
Normal file
7
.changeset/fn-7777-skills-content-not-found.md
Normal file
@@ -0,0 +1,7 @@
|
|||||||
|
---
|
||||||
|
"@runfusion/fusion": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
summary: Fix Skills view showing "Skill not found" when opening any skill's content.
|
||||||
|
category: fix
|
||||||
|
dev: The /skills/:id/content and /skills/:id/file routes double-decoded the URL param (Express 5 already decodes route params once), corrupting the encoded source segment so the id no longer matched computeSkillId's discovery output. Routes now use the once-decoded canonical id (FN-7777).
|
||||||
@@ -3,7 +3,7 @@ import { EventEmitter } from "node:events";
|
|||||||
import type { Task } from "@fusion/core";
|
import type { Task } from "@fusion/core";
|
||||||
import { request } from "../test-request.js";
|
import { request } from "../test-request.js";
|
||||||
import { createServer } from "../server.js";
|
import { createServer } from "../server.js";
|
||||||
import type { SkillsAdapter } from "../skills-adapter.js";
|
import type { DiscoveredSkill, SkillsAdapter } from "../skills-adapter.js";
|
||||||
import { computeSkillId, parseSkillId } from "../skills-adapter.js";
|
import { computeSkillId, parseSkillId } from "../skills-adapter.js";
|
||||||
|
|
||||||
class MockStore extends EventEmitter {
|
class MockStore extends EventEmitter {
|
||||||
@@ -51,6 +51,60 @@ class MockStore extends EventEmitter {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Mock skills adapter for testing
|
// Mock skills adapter for testing
|
||||||
|
function createExactLookupSkillsAdapter(skills: DiscoveredSkill[]): SkillsAdapter {
|
||||||
|
return createMockSkillsAdapter({
|
||||||
|
discoverSkills: vi.fn().mockResolvedValue(skills),
|
||||||
|
readSkillContent: vi.fn().mockImplementation(async (_rootDir: string, skillId: string) => {
|
||||||
|
if (!parseSkillId(skillId)) {
|
||||||
|
throw new Error(`Invalid skill ID format: ${skillId}`);
|
||||||
|
}
|
||||||
|
const skill = skills.find((entry) => entry.id === skillId);
|
||||||
|
if (!skill) {
|
||||||
|
throw new Error(`Skill not found: ${skillId}`);
|
||||||
|
}
|
||||||
|
return {
|
||||||
|
name: skill.name,
|
||||||
|
skillMd: `# ${skill.name}\n\nCanonical id: ${skill.id}`,
|
||||||
|
files: [{ name: "notes.txt", relativePath: "notes.txt", type: "file" as const }],
|
||||||
|
};
|
||||||
|
}),
|
||||||
|
readSkillFileContent: vi.fn().mockImplementation(async (_rootDir: string, skillId: string, relativePath: string) => {
|
||||||
|
if (!parseSkillId(skillId)) {
|
||||||
|
throw new Error(`Invalid skill ID format: ${skillId}`);
|
||||||
|
}
|
||||||
|
const skill = skills.find((entry) => entry.id === skillId);
|
||||||
|
if (!skill) {
|
||||||
|
throw new Error(`Skill not found: ${skillId}`);
|
||||||
|
}
|
||||||
|
if (relativePath !== "notes.txt") {
|
||||||
|
throw new Error(`Skill file not found: ${relativePath}`);
|
||||||
|
}
|
||||||
|
return {
|
||||||
|
name: "notes.txt",
|
||||||
|
relativePath,
|
||||||
|
content: `Supplement for ${skill.id}`,
|
||||||
|
isText: true,
|
||||||
|
};
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
function createDiscoveredSkill(source: string, relativePath: string): DiscoveredSkill {
|
||||||
|
return {
|
||||||
|
id: computeSkillId(source, relativePath),
|
||||||
|
name: relativePath.replace(/^skills\//, ""),
|
||||||
|
path: `/tmp/fn-skills/${relativePath}`,
|
||||||
|
relativePath,
|
||||||
|
enabled: true,
|
||||||
|
metadata: {
|
||||||
|
source,
|
||||||
|
scope: "project",
|
||||||
|
origin: source === "*" ? "top-level" : "package",
|
||||||
|
baseDir: "/tmp/fn-skills",
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
function createMockSkillsAdapter(overrides?: Partial<SkillsAdapter>): SkillsAdapter {
|
function createMockSkillsAdapter(overrides?: Partial<SkillsAdapter>): SkillsAdapter {
|
||||||
return {
|
return {
|
||||||
discoverSkills: vi.fn().mockResolvedValue([
|
discoverSkills: vi.fn().mockResolvedValue([
|
||||||
@@ -246,10 +300,32 @@ describe("Skills routes", () => {
|
|||||||
});
|
});
|
||||||
expect(mockAdapter.readSkillContent).toHaveBeenCalledWith(
|
expect(mockAdapter.readSkillContent).toHaveBeenCalledWith(
|
||||||
"/tmp/fn-skills",
|
"/tmp/fn-skills",
|
||||||
"npm:@example/skill::skills/example/SKILL.md",
|
"npm%3A%40example%2Fskill::skills/example/SKILL.md",
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it.each([
|
||||||
|
["disk source", createDiscoveredSkill("*", "skills/local/SKILL.md")],
|
||||||
|
["plugin source", createDiscoveredSkill("plugin:foo", "skills/bar/SKILL.md")],
|
||||||
|
["npm scoped source", createDiscoveredSkill("npm:@example/skill", "skills/example/SKILL.md")],
|
||||||
|
])("preserves the canonical ID for %s content lookup", async (_label, skill) => {
|
||||||
|
const mockAdapter = createExactLookupSkillsAdapter([skill]);
|
||||||
|
const store = new MockStore();
|
||||||
|
const app = createServer(store as any, { skillsAdapter: mockAdapter as SkillsAdapter });
|
||||||
|
|
||||||
|
const res = await request(app, "GET", `/api/skills/${encodeURIComponent(skill.id)}/content`);
|
||||||
|
|
||||||
|
expect(res.status).toBe(200);
|
||||||
|
expect(res.body).toEqual({
|
||||||
|
content: {
|
||||||
|
name: skill.name,
|
||||||
|
skillMd: `# ${skill.name}\n\nCanonical id: ${skill.id}`,
|
||||||
|
files: [{ name: "notes.txt", relativePath: "notes.txt", type: "file" }],
|
||||||
|
},
|
||||||
|
});
|
||||||
|
expect(mockAdapter.readSkillContent).toHaveBeenCalledWith("/tmp/fn-skills", skill.id);
|
||||||
|
});
|
||||||
|
|
||||||
it("returns 404 when skills adapter is not configured", async () => {
|
it("returns 404 when skills adapter is not configured", async () => {
|
||||||
const store = new MockStore();
|
const store = new MockStore();
|
||||||
const app = createServer(store as any, {});
|
const app = createServer(store as any, {});
|
||||||
@@ -296,6 +372,70 @@ describe("Skills routes", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("GET /api/skills/:id/file", () => {
|
||||||
|
it.each([
|
||||||
|
["disk source", createDiscoveredSkill("*", "skills/local/SKILL.md")],
|
||||||
|
["plugin source", createDiscoveredSkill("plugin:foo", "skills/bar/SKILL.md")],
|
||||||
|
["npm scoped source", createDiscoveredSkill("npm:@example/skill", "skills/example/SKILL.md")],
|
||||||
|
])("preserves the canonical ID for %s file lookup", async (_label, skill) => {
|
||||||
|
const mockAdapter = createExactLookupSkillsAdapter([skill]);
|
||||||
|
const store = new MockStore();
|
||||||
|
const app = createServer(store as any, { skillsAdapter: mockAdapter as SkillsAdapter });
|
||||||
|
|
||||||
|
const res = await request(app, "GET", `/api/skills/${encodeURIComponent(skill.id)}/file?path=${encodeURIComponent("notes.txt")}`);
|
||||||
|
|
||||||
|
expect(res.status).toBe(200);
|
||||||
|
expect(res.body).toEqual({
|
||||||
|
file: {
|
||||||
|
name: "notes.txt",
|
||||||
|
relativePath: "notes.txt",
|
||||||
|
content: `Supplement for ${skill.id}`,
|
||||||
|
isText: true,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
expect(mockAdapter.readSkillFileContent).toHaveBeenCalledWith("/tmp/fn-skills", skill.id, "notes.txt");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns 400 when path is missing", async () => {
|
||||||
|
const skill = createDiscoveredSkill("plugin:foo", "skills/bar/SKILL.md");
|
||||||
|
const mockAdapter = createExactLookupSkillsAdapter([skill]);
|
||||||
|
const store = new MockStore();
|
||||||
|
const app = createServer(store as any, { skillsAdapter: mockAdapter as SkillsAdapter });
|
||||||
|
|
||||||
|
const res = await request(app, "GET", `/api/skills/${encodeURIComponent(skill.id)}/file`);
|
||||||
|
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
expect(res.body).toEqual({ error: "path is required", code: "invalid_path" });
|
||||||
|
expect(mockAdapter.readSkillFileContent).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns 400 for invalid skill IDs", async () => {
|
||||||
|
const mockAdapter = createExactLookupSkillsAdapter([]);
|
||||||
|
const store = new MockStore();
|
||||||
|
const app = createServer(store as any, { skillsAdapter: mockAdapter as SkillsAdapter });
|
||||||
|
|
||||||
|
const res = await request(app, "GET", `/api/skills/${encodeURIComponent("invalid")}/file?path=notes.txt`);
|
||||||
|
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
expect(res.body).toEqual({
|
||||||
|
error: "Invalid skill ID format: invalid",
|
||||||
|
code: "invalid_path",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns 404 for non-existent skill files", async () => {
|
||||||
|
const skill = createDiscoveredSkill("plugin:foo", "skills/bar/SKILL.md");
|
||||||
|
const mockAdapter = createExactLookupSkillsAdapter([skill]);
|
||||||
|
const store = new MockStore();
|
||||||
|
const app = createServer(store as any, { skillsAdapter: mockAdapter as SkillsAdapter });
|
||||||
|
|
||||||
|
const res = await request(app, "GET", `/api/skills/${encodeURIComponent(skill.id)}/file?path=${encodeURIComponent("missing.txt")}`);
|
||||||
|
|
||||||
|
expect(res.status).toBe(404);
|
||||||
|
expect(res.body).toEqual({ error: "Skill file not found", code: "skill_file_not_found" });
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("PATCH /api/skills/execution", () => {
|
describe("PATCH /api/skills/execution", () => {
|
||||||
it("toggles skill execution successfully", async () => {
|
it("toggles skill execution successfully", async () => {
|
||||||
const mockAdapter = createMockSkillsAdapter();
|
const mockAdapter = createMockSkillsAdapter();
|
||||||
|
|||||||
@@ -50,14 +50,11 @@ export function registerAgentSkillsRoutes(ctx: ApiRoutesContext): void {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
const encodedSkillId = req.params.id as string;
|
/*
|
||||||
let skillId = encodedSkillId;
|
FNXC:Skills 2026-07-10-00:00:
|
||||||
try {
|
Express 5 decodes route params once before this handler runs. Discovery IDs from computeSkillId intentionally keep only the source segment URL-encoded, so pass req.params.id through unchanged; a second decode corrupts plugin/npm/path sources and breaks exact-ID skill content lookup (FN-7777).
|
||||||
skillId = decodeURIComponent(encodedSkillId);
|
*/
|
||||||
} catch {
|
const skillId = req.params.id as string;
|
||||||
res.status(400).json({ error: "Invalid skill ID", code: "invalid_skill_id" });
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
const rootDir = scopedStore.getRootDir();
|
const rootDir = scopedStore.getRootDir();
|
||||||
const content = await skillsAdapter.readSkillContent(rootDir, skillId);
|
const content = await skillsAdapter.readSkillContent(rootDir, skillId);
|
||||||
@@ -97,14 +94,11 @@ export function registerAgentSkillsRoutes(ctx: ApiRoutesContext): void {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
const encodedSkillId = req.params.id as string;
|
/*
|
||||||
let skillId = encodedSkillId;
|
FNXC:Skills 2026-07-10-00:00:
|
||||||
try {
|
Express 5 decodes route params once before this handler runs. Discovery IDs from computeSkillId intentionally keep only the source segment URL-encoded, so pass req.params.id through unchanged; a second decode corrupts plugin/npm/path sources and breaks exact-ID supplementary file lookup (FN-7777).
|
||||||
skillId = decodeURIComponent(encodedSkillId);
|
*/
|
||||||
} catch {
|
const skillId = req.params.id as string;
|
||||||
res.status(400).json({ error: "Invalid skill ID", code: "invalid_skill_id" });
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
const rawPath = typeof req.query.path === "string" ? req.query.path : "";
|
const rawPath = typeof req.query.path === "string" ? req.query.path : "";
|
||||||
if (!rawPath.trim()) {
|
if (!rawPath.trim()) {
|
||||||
|
|||||||
Reference in New Issue
Block a user