feat(FN-1913): add skill content viewer to SkillsView
- Add /api/skills/:id/content support in dashboard API routes and client exports for skill content types - Extend the skills adapter with readSkillContent() to read SKILL.md and list supplementary files while handling invalid and missing skills - Update SkillsView with expandable skill detail panels, lazy content loading, and loading/error/empty states without interfering with enable toggles - Add comprehensive SkillsView, mobile CSS, and skills-adapter tests covering interaction flows and content parsing
This commit is contained in:
@@ -1,5 +1,9 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { createSkillsAdapter } from "../skills-adapter.js";
|
||||
import { writeFile, mkdir, access } from "node:fs/promises";
|
||||
import { join, dirname } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
import { rm } from "node:fs/promises";
|
||||
|
||||
describe("createSkillsAdapter - fetchCatalog fallback behavior", () => {
|
||||
const originalFetch = globalThis.fetch;
|
||||
@@ -212,3 +216,218 @@ describe("createSkillsAdapter - fetchCatalog fallback behavior", () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("createSkillsAdapter - readSkillContent", () => {
|
||||
const originalFetch = globalThis.fetch;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
globalThis.fetch = originalFetch;
|
||||
});
|
||||
|
||||
async function createMockSkillDir(skillMdContent?: string, extraFiles?: string[]) {
|
||||
const skillDir = join(tmpdir(), `skill-test-${Date.now()}-${Math.random().toString(36).slice(2)}`);
|
||||
await mkdir(skillDir, { recursive: true });
|
||||
|
||||
if (skillMdContent !== undefined) {
|
||||
await writeFile(join(skillDir, "SKILL.md"), skillMdContent, "utf-8");
|
||||
}
|
||||
|
||||
if (extraFiles) {
|
||||
for (const file of extraFiles) {
|
||||
const filePath = join(skillDir, file);
|
||||
const fileDir = dirname(filePath);
|
||||
if (!await access(fileDir).then(() => true).catch(() => false)) {
|
||||
await mkdir(fileDir, { recursive: true });
|
||||
}
|
||||
await writeFile(filePath, `content of ${file}`, "utf-8");
|
||||
}
|
||||
}
|
||||
|
||||
return skillDir;
|
||||
}
|
||||
|
||||
async function cleanup(skillDir: string) {
|
||||
try {
|
||||
await rm(skillDir, { recursive: true, force: true });
|
||||
} catch {
|
||||
// Ignore cleanup errors
|
||||
}
|
||||
}
|
||||
|
||||
it("returns SKILL.md content and file listing for a valid skill", async () => {
|
||||
const skillDir = await createMockSkillDir(
|
||||
"# Test Skill\n\nThis is a test skill.",
|
||||
["references/ref.md", "workflows/test.sh"]
|
||||
);
|
||||
|
||||
const adapter = createSkillsAdapter({
|
||||
packageManager: {
|
||||
resolve: vi.fn().mockResolvedValue({ skills: [] }),
|
||||
},
|
||||
getSettingsPath: vi.fn().mockReturnValue("/tmp/settings.json"),
|
||||
});
|
||||
|
||||
// Spy on discoverSkills to return a controlled skill
|
||||
const mockDiscoveredSkill = {
|
||||
id: "npm::skills/test-skill",
|
||||
name: "test-skill",
|
||||
path: join(skillDir, "SKILL.md"),
|
||||
relativePath: "skills/test-skill",
|
||||
enabled: true,
|
||||
metadata: {
|
||||
source: "npm",
|
||||
scope: "project" as const,
|
||||
origin: "top-level" as const,
|
||||
baseDir: skillDir,
|
||||
},
|
||||
};
|
||||
|
||||
vi.spyOn(adapter, "discoverSkills").mockResolvedValue([mockDiscoveredSkill]);
|
||||
|
||||
const result = await adapter.readSkillContent("/project", "npm::skills/test-skill");
|
||||
|
||||
expect(result.name).toBe("test-skill");
|
||||
expect(result.skillMd).toBe("# Test Skill\n\nThis is a test skill.");
|
||||
expect(result.files).toHaveLength(2);
|
||||
expect(result.files.map((f) => f.name).sort()).toEqual(["references", "workflows"]);
|
||||
expect(result.files.find((f) => f.name === "references")!.type).toBe("directory");
|
||||
expect(result.files.find((f) => f.name === "workflows")!.type).toBe("directory");
|
||||
|
||||
await cleanup(skillDir);
|
||||
});
|
||||
|
||||
it("returns empty skillMd when SKILL.md doesn't exist", async () => {
|
||||
const skillDir = await createMockSkillDir(undefined, ["readme.txt"]);
|
||||
|
||||
const adapter = createSkillsAdapter({
|
||||
packageManager: {
|
||||
resolve: vi.fn().mockResolvedValue({ skills: [] }),
|
||||
},
|
||||
getSettingsPath: vi.fn().mockReturnValue("/tmp/settings.json"),
|
||||
});
|
||||
|
||||
const mockDiscoveredSkill = {
|
||||
id: "npm::skills/test-skill",
|
||||
name: "test-skill",
|
||||
path: skillDir,
|
||||
relativePath: "skills/test-skill",
|
||||
enabled: true,
|
||||
metadata: {
|
||||
source: "npm",
|
||||
scope: "project" as const,
|
||||
origin: "top-level" as const,
|
||||
},
|
||||
};
|
||||
|
||||
vi.spyOn(adapter, "discoverSkills").mockResolvedValue([mockDiscoveredSkill]);
|
||||
|
||||
const result = await adapter.readSkillContent("/project", "npm::skills/test-skill");
|
||||
|
||||
expect(result.name).toBe("test-skill");
|
||||
expect(result.skillMd).toBe("");
|
||||
expect(result.files).toHaveLength(1);
|
||||
expect(result.files[0]!.name).toBe("readme.txt");
|
||||
|
||||
await cleanup(skillDir);
|
||||
});
|
||||
|
||||
it("throws error for invalid skill ID format", async () => {
|
||||
const adapter = createSkillsAdapter({
|
||||
packageManager: { resolve: vi.fn().mockResolvedValue({ skills: [] }) },
|
||||
getSettingsPath: vi.fn().mockReturnValue("/tmp/settings.json"),
|
||||
});
|
||||
|
||||
await expect(adapter.readSkillContent("/project", "invalid-skill-id")).rejects.toThrow(
|
||||
"Invalid skill ID format"
|
||||
);
|
||||
});
|
||||
|
||||
it("throws error for non-existent skill", async () => {
|
||||
const adapter = createSkillsAdapter({
|
||||
packageManager: { resolve: vi.fn().mockResolvedValue({ skills: [] }) },
|
||||
getSettingsPath: vi.fn().mockReturnValue("/tmp/settings.json"),
|
||||
});
|
||||
|
||||
await expect(adapter.readSkillContent("/project", "npm::skills/nonexistent")).rejects.toThrow(
|
||||
"Skill not found"
|
||||
);
|
||||
});
|
||||
|
||||
it("filters out SKILL.md from supplementary files listing", async () => {
|
||||
const skillDir = await createMockSkillDir(
|
||||
"# Test Skill",
|
||||
["SKILL.md", "readme.txt"]
|
||||
);
|
||||
|
||||
const adapter = createSkillsAdapter({
|
||||
packageManager: {
|
||||
resolve: vi.fn().mockResolvedValue({ skills: [] }),
|
||||
},
|
||||
getSettingsPath: vi.fn().mockReturnValue("/tmp/settings.json"),
|
||||
});
|
||||
|
||||
const mockDiscoveredSkill = {
|
||||
id: "npm::skills/test-skill",
|
||||
name: "test-skill",
|
||||
path: join(skillDir, "SKILL.md"),
|
||||
relativePath: "skills/test-skill",
|
||||
enabled: true,
|
||||
metadata: {
|
||||
source: "npm",
|
||||
scope: "project" as const,
|
||||
origin: "top-level" as const,
|
||||
},
|
||||
};
|
||||
|
||||
vi.spyOn(adapter, "discoverSkills").mockResolvedValue([mockDiscoveredSkill]);
|
||||
|
||||
const result = await adapter.readSkillContent("/project", "npm::skills/test-skill");
|
||||
|
||||
// Should only have readme.txt, not SKILL.md
|
||||
expect(result.files).toHaveLength(1);
|
||||
expect(result.files[0]!.name).toBe("readme.txt");
|
||||
|
||||
await cleanup(skillDir);
|
||||
});
|
||||
|
||||
it("handles skill path that is already a directory", async () => {
|
||||
const skillDir = await createMockSkillDir(
|
||||
"# Test Skill",
|
||||
["readme.txt"]
|
||||
);
|
||||
|
||||
const adapter = createSkillsAdapter({
|
||||
packageManager: {
|
||||
resolve: vi.fn().mockResolvedValue({ skills: [] }),
|
||||
},
|
||||
getSettingsPath: vi.fn().mockReturnValue("/tmp/settings.json"),
|
||||
});
|
||||
|
||||
const mockDiscoveredSkill = {
|
||||
id: "npm::skills/test-skill",
|
||||
name: "test-skill",
|
||||
path: skillDir, // Path is already a directory
|
||||
relativePath: "skills/test-skill",
|
||||
enabled: true,
|
||||
metadata: {
|
||||
source: "npm",
|
||||
scope: "project" as const,
|
||||
origin: "top-level" as const,
|
||||
},
|
||||
};
|
||||
|
||||
vi.spyOn(adapter, "discoverSkills").mockResolvedValue([mockDiscoveredSkill]);
|
||||
|
||||
const result = await adapter.readSkillContent("/project", "npm::skills/test-skill");
|
||||
|
||||
expect(result.name).toBe("test-skill");
|
||||
expect(result.skillMd).toBe("# Test Skill");
|
||||
expect(result.files).toHaveLength(1);
|
||||
|
||||
await cleanup(skillDir);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
export { createServer, loadTlsCredentialsFromEnv, type ServerOptions } from "./server.js";
|
||||
export { createSkillsAdapter, getProjectSettingsPath, type SkillsAdapter, type DiscoveredSkill, type CatalogEntry, type CatalogFetchResult, type ToggleSkillResult, type UpstreamError, type UpstreamErrorCode } from "./skills-adapter.js";
|
||||
export { createSkillsAdapter, getProjectSettingsPath, type SkillsAdapter, type DiscoveredSkill, type CatalogEntry, type CatalogFetchResult, type ToggleSkillResult, type UpstreamError, type UpstreamErrorCode, type SkillContent, type SkillFileEntry } from "./skills-adapter.js";
|
||||
export { GitHubClient, isPrMergeReady, type PrMergeStatus, type PrCheckStatus, type ReviewDecision, type MergePrParams, type FindPrParams } from "./github.js";
|
||||
export { rateLimit, RATE_LIMITS, type RateLimitOptions } from "./rate-limit.js";
|
||||
export { GitHubPollingService, type GitHubPollingServiceOptions, type TaskWatchInput, type WatchedBadgeType } from "./github-poll.js";
|
||||
|
||||
@@ -17043,6 +17043,48 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* GET /api/skills/:id/content
|
||||
* Read the contents of a skill's SKILL.md file and list supplementary files.
|
||||
* Params: id (URL-encoded skill ID)
|
||||
* Query: projectId (optional) for multi-project context
|
||||
* Response: { content: SkillContent }
|
||||
* Error: 404 { error: string; code: "skill_not_found" | "adapter_not_configured" }
|
||||
*/
|
||||
router.get("/skills/:id/content", async (req, res) => {
|
||||
try {
|
||||
const scopedStore = await getScopedStore(req);
|
||||
const skillsAdapter = options?.skillsAdapter;
|
||||
|
||||
if (!skillsAdapter) {
|
||||
res.status(404).json({ error: "Skills adapter not configured", code: "adapter_not_configured" });
|
||||
return;
|
||||
}
|
||||
|
||||
const rootDir = scopedStore.getRootDir();
|
||||
const skillId = req.params.id as string;
|
||||
|
||||
const content = await skillsAdapter.readSkillContent(rootDir, skillId);
|
||||
|
||||
res.json({ content });
|
||||
} catch (err: unknown) {
|
||||
if (err instanceof ApiError) {
|
||||
throw err;
|
||||
}
|
||||
if (err instanceof Error) {
|
||||
if (err.message.includes("Invalid skill ID")) {
|
||||
res.status(400).json({ error: err.message, code: "invalid_skill_id" });
|
||||
return;
|
||||
}
|
||||
if (err.message.includes("Skill not found")) {
|
||||
res.status(404).json({ error: err.message, code: "skill_not_found" });
|
||||
return;
|
||||
}
|
||||
}
|
||||
rethrowAsApiError(err, "Failed to read skill content");
|
||||
}
|
||||
});
|
||||
|
||||
// ── Remote Node Proxy Routes ───────────────────────────────────────────
|
||||
|
||||
/** GET /api/proxy/:nodeId/health — Forward health check to remote node */
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
*/
|
||||
|
||||
import { access } from "node:fs/promises";
|
||||
import { readFile, writeFile, mkdir } from "node:fs/promises";
|
||||
import { readFile, writeFile, mkdir, readdir } from "node:fs/promises";
|
||||
import { join, relative, dirname } from "node:path";
|
||||
|
||||
/**
|
||||
@@ -100,6 +100,24 @@ export interface ToggleSkillResult {
|
||||
targetFile: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* A file entry in a skill directory.
|
||||
*/
|
||||
export interface SkillFileEntry {
|
||||
name: string;
|
||||
relativePath: string;
|
||||
type: "file" | "directory";
|
||||
}
|
||||
|
||||
/**
|
||||
* Content of a skill including its SKILL.md and supplementary files.
|
||||
*/
|
||||
export interface SkillContent {
|
||||
name: string;
|
||||
skillMd: string;
|
||||
files: SkillFileEntry[];
|
||||
}
|
||||
|
||||
/**
|
||||
* Upstream error codes for catalog fetch failures.
|
||||
*/
|
||||
@@ -136,6 +154,11 @@ export interface SkillsAdapter {
|
||||
* Fetch the skills.sh catalog with optional authentication.
|
||||
*/
|
||||
fetchCatalog(input: { limit: number; query?: string }): Promise<CatalogFetchResult | UpstreamError>;
|
||||
|
||||
/**
|
||||
* Read the contents of a skill's SKILL.md file and list supplementary files.
|
||||
*/
|
||||
readSkillContent(rootDir: string, skillId: string): Promise<SkillContent>;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -454,6 +477,78 @@ export function createSkillsAdapter(options: {
|
||||
};
|
||||
}
|
||||
},
|
||||
|
||||
async readSkillContent(rootDir: string, skillId: string): Promise<SkillContent> {
|
||||
// Parse the skill ID to get source and relativePath
|
||||
const parsed = parseSkillId(skillId);
|
||||
if (!parsed) {
|
||||
throw new Error(`Invalid skill ID format: ${skillId}`);
|
||||
}
|
||||
|
||||
// Find the skill in discovered skills to get its path
|
||||
const discovered = await this.discoverSkills(rootDir);
|
||||
const skill = discovered.find((s) => s.id === skillId);
|
||||
if (!skill) {
|
||||
throw new Error(`Skill not found: ${skillId}`);
|
||||
}
|
||||
|
||||
// Determine the skill directory
|
||||
// If path points to a file (e.g., SKILL.md), use dirname(path)
|
||||
// If path points to a directory, use it directly
|
||||
let skillDir = skill.path;
|
||||
try {
|
||||
const stat = await access(skill.path);
|
||||
// Path exists, check if it's a file or directory
|
||||
// We can't easily check with access(), so we try to read as file first
|
||||
try {
|
||||
await readFile(skill.path, "utf-8");
|
||||
// It's a file, get the directory
|
||||
skillDir = dirname(skill.path);
|
||||
} catch {
|
||||
// Not a file (might be a directory or error), use path as-is
|
||||
skillDir = skill.path;
|
||||
}
|
||||
} catch {
|
||||
// Path doesn't exist, use dirname as fallback
|
||||
skillDir = dirname(skill.path);
|
||||
}
|
||||
|
||||
// Read SKILL.md from the skill directory
|
||||
const skillMdPath = join(skillDir, "SKILL.md");
|
||||
let skillMd = "";
|
||||
if (await pathExists(skillMdPath)) {
|
||||
try {
|
||||
skillMd = await readFile(skillMdPath, "utf-8");
|
||||
} catch {
|
||||
// Ignore read errors, keep empty string
|
||||
}
|
||||
}
|
||||
|
||||
// List files in the skill directory (non-recursive for MVP)
|
||||
const files: SkillFileEntry[] = [];
|
||||
try {
|
||||
const entries = await readdir(skillDir, { withFileTypes: true });
|
||||
for (const entry of entries) {
|
||||
// Skip SKILL.md as it's shown separately
|
||||
if (entry.name === "SKILL.md") {
|
||||
continue;
|
||||
}
|
||||
files.push({
|
||||
name: entry.name,
|
||||
relativePath: entry.name,
|
||||
type: entry.isDirectory() ? "directory" : "file",
|
||||
});
|
||||
}
|
||||
} catch {
|
||||
// Ignore readdir errors, return empty files array
|
||||
}
|
||||
|
||||
return {
|
||||
name: skill.name,
|
||||
skillMd,
|
||||
files,
|
||||
};
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user