From 2758dde301e5efb1167a496a5fd0aa77db610155 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 10 Jul 2026 07:37:18 -0700 Subject: [PATCH] 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) --- .../fn-7777-skills-content-not-found.md | 7 + .../src/__tests__/routes-skills.test.ts | 144 +++++++++++++++++- .../routes/register-agent-skills-routes.ts | 26 ++-- 3 files changed, 159 insertions(+), 18 deletions(-) create mode 100644 .changeset/fn-7777-skills-content-not-found.md diff --git a/.changeset/fn-7777-skills-content-not-found.md b/.changeset/fn-7777-skills-content-not-found.md new file mode 100644 index 0000000000..462fade828 --- /dev/null +++ b/.changeset/fn-7777-skills-content-not-found.md @@ -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). diff --git a/packages/dashboard/src/__tests__/routes-skills.test.ts b/packages/dashboard/src/__tests__/routes-skills.test.ts index 3c9e7533ab..580d4cd71c 100644 --- a/packages/dashboard/src/__tests__/routes-skills.test.ts +++ b/packages/dashboard/src/__tests__/routes-skills.test.ts @@ -3,7 +3,7 @@ import { EventEmitter } from "node:events"; import type { Task } from "@fusion/core"; import { request } from "../test-request.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"; class MockStore extends EventEmitter { @@ -51,6 +51,60 @@ class MockStore extends EventEmitter { } // 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 { return { discoverSkills: vi.fn().mockResolvedValue([ @@ -246,10 +300,32 @@ describe("Skills routes", () => { }); expect(mockAdapter.readSkillContent).toHaveBeenCalledWith( "/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 () => { const store = new MockStore(); 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", () => { it("toggles skill execution successfully", async () => { const mockAdapter = createMockSkillsAdapter(); diff --git a/packages/dashboard/src/routes/register-agent-skills-routes.ts b/packages/dashboard/src/routes/register-agent-skills-routes.ts index e135f5806b..58a6881104 100644 --- a/packages/dashboard/src/routes/register-agent-skills-routes.ts +++ b/packages/dashboard/src/routes/register-agent-skills-routes.ts @@ -50,14 +50,11 @@ export function registerAgentSkillsRoutes(ctx: ApiRoutesContext): void { return; } - const encodedSkillId = req.params.id as string; - let skillId = encodedSkillId; - try { - skillId = decodeURIComponent(encodedSkillId); - } catch { - res.status(400).json({ error: "Invalid skill ID", code: "invalid_skill_id" }); - return; - } + /* + FNXC:Skills 2026-07-10-00:00: + 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). + */ + const skillId = req.params.id as string; const rootDir = scopedStore.getRootDir(); const content = await skillsAdapter.readSkillContent(rootDir, skillId); @@ -97,14 +94,11 @@ export function registerAgentSkillsRoutes(ctx: ApiRoutesContext): void { return; } - const encodedSkillId = req.params.id as string; - let skillId = encodedSkillId; - try { - skillId = decodeURIComponent(encodedSkillId); - } catch { - res.status(400).json({ error: "Invalid skill ID", code: "invalid_skill_id" }); - return; - } + /* + FNXC:Skills 2026-07-10-00:00: + 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). + */ + const skillId = req.params.id as string; const rawPath = typeof req.query.path === "string" ? req.query.path : ""; if (!rawPath.trim()) {