From 51c3f0c62a4fb69af0d046656ee0c848cc3764df Mon Sep 17 00:00:00 2001 From: ekko <152005280+EKKOLearnAI@users.noreply.github.com> Date: Wed, 20 May 2026 15:38:56 +0800 Subject: [PATCH] [codex] harden recursive skill scan (#876) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: recursive skill scan for nested sub-category directories The Web UI skill scanner (scanSkillsDir) only checked one level deep: skills///SKILL.md. Sub-category containers like mlops/evaluation/ (which has DESCRIPTION.md + subdirs but no SKILL.md) were skipped entirely, hiding all 12 nested skills under mlops. Changes: - scanSkillsDir: extract collectSkills() recursive helper that depth-first searches for SKILL.md at every level under a category directory. Directories without SKILL.md but with subdirectories are recursively descended into. - listFiles handler: replace hardcoded join(category, skill) path with recursive findSkillDir() search, so nested skill file browsing works (e.g., mlops/evaluation/lm-evaluation-harness). Fixes mlops category showing 1 skill instead of 13. All 20 other categories verified with zero regression. * fix: pA handler also needs recursive search for nested skill file content The readFile_ (pA) handler was constructing direct paths like skills/category/skill/... which fails for nested sub-category skills (mlops/evaluation/lm-evaluation-harness). Added fallback recursive search when direct path returns 404. Also fixed listFiles (sA) handler to use recursive search for the same reason - previous fix to dist was not in source TS. Verified: - lm-evaluation-harness SKILL.md content: 200 ✅ - vllm SKILL.md: 200 ✅ - huggingface-hub (non-nested): 200 ✅ - reference file in nested skill: 200 ✅ * fix: pA handler also needs recursive search for nested skill file content The readFile_ (pA) handler was constructing direct paths like skills/category/skill/... which fails for nested sub-category skills (mlops/evaluation/lm-evaluation-harness). Added fallback recursive search when direct path returns 404. Also fixed listFiles (sA) handler to use recursive search for the same reason - previous fix to dist was not in source TS. Verified: - lm-evaluation-harness SKILL.md content: 200 ✅ - vllm SKILL.md: 200 ✅ - huggingface-hub (non-nested): 200 ✅ - reference file in nested skill: 200 ✅ * harden recursive skill lookup --------- Co-authored-by: gs Co-authored-by: gutanulaif --- .../server/src/controllers/hermes/skills.ts | 133 ++++++++++++++---- 1 file changed, 104 insertions(+), 29 deletions(-) diff --git a/packages/server/src/controllers/hermes/skills.ts b/packages/server/src/controllers/hermes/skills.ts index d89c93e..5122fd6 100644 --- a/packages/server/src/controllers/hermes/skills.ts +++ b/packages/server/src/controllers/hermes/skills.ts @@ -84,6 +84,32 @@ function readUsageStats(usageContent: string | null): Map { return map } +async function findSkillDirByName(rootDir: string, skillName: string): Promise { + let entries: import('fs').Dirent[] + try { + entries = await readdir(rootDir, { withFileTypes: true }) + } catch { + return null + } + + for (const entry of entries) { + if (!entry.isDirectory() || entry.name.startsWith('.')) continue + + const entryPath = join(rootDir, entry.name) + const skillMd = await safeReadFile(join(entryPath, 'SKILL.md')) + if (skillMd !== null) { + if (entry.name === skillName) return entryPath + // This is another skill root. Do not search inside its references/scripts. + continue + } + + const found = await findSkillDirByName(entryPath, skillName) + if (found) return found + } + + return null +} + /** * Scan for skills at different directory depths. * @@ -136,33 +162,45 @@ async function scanSkillsDir(skillsDir: string, bundledManifest: Map { + const entries = await readdir(dir, { withFileTypes: true }) + const results: any[] = [] + for (const entry of entries) { + if (!entry.isDirectory() || entry.name.startsWith('.')) continue + const entryPath = join(dir, entry.name) + const skillMd = await safeReadFile(join(entryPath, 'SKILL.md')) + if (skillMd) { + const source = getSkillSource(entry.name, bundledManifest, hubNames) + let modified = false + if (source === 'builtin') { + const manifestHash = bundledManifest.get(entry.name) + if (manifestHash) { + const currentHash = await dirHash(entryPath) + modified = currentHash !== manifestHash + } } + const usage = usageStats.get(entry.name) + results.push({ + name: entry.name, + description: extractDescription(skillMd), + enabled: !disabledList.includes(entry.name), + source, + modified: modified || undefined, + patchCount: usage?.patch_count, + useCount: usage?.use_count, + viewCount: usage?.view_count, + pinned: usage?.pinned || undefined, + }) + } else { + // No SKILL.md — might be a sub-category container, recurse deeper + const subResults = await collectSkills(entryPath) + results.push(...subResults) } - const usage = usageStats.get(se.name) - skills.push({ - name: se.name, - description: extractDescription(skillMd), - enabled: !disabledList.includes(se.name), - source, - modified: modified || undefined, - patchCount: usage?.patch_count, - useCount: usage?.use_count, - viewCount: usage?.view_count, - pinned: usage?.pinned || undefined, - }) } + return results } + skills.push(...await collectSkills(catDir)) if (skills.length > 0) { categories.push({ name: cat.name, description: cat.description, skills }) } @@ -280,12 +318,29 @@ export async function toggle(ctx: any) { export async function listFiles(ctx: any) { const { category, skill } = ctx.params const hd = getHermesDir() - // Handle "misc" category: real skill dir is skills/, not skills/misc/ - const realDir = category === 'misc' ? skill : join(category, skill) - const skillDir = join(hd, 'skills', realDir) + const skillsDir = join(hd, 'skills', category) + if (category === 'misc') { + const skillDir = join(hd, 'skills', skill) + try { + const allFiles = await listFilesRecursive(skillDir, '') + const files = allFiles.filter((f: any) => f.path !== 'SKILL.md') + ctx.body = { files } + } catch (err: any) { + ctx.status = 500 + ctx.body = { error: err.message } + } + return + } + // Recursively find the actual skill directory (supports nested sub-categories like mlops/evaluation/lm-evaluation-harness) try { + const skillDir = await findSkillDirByName(skillsDir, skill) + if (!skillDir) { + ctx.status = 404 + ctx.body = { error: 'Skill not found' } + return + } const allFiles = await listFilesRecursive(skillDir, '') - const files = allFiles.filter(f => f.path !== 'SKILL.md') + const files = allFiles.filter((f: any) => f.path !== 'SKILL.md') ctx.body = { files } } catch (err: any) { ctx.status = 500 @@ -296,7 +351,7 @@ export async function listFiles(ctx: any) { export async function readFile_(ctx: any) { const filePath = (ctx.params as any).path const hd = getHermesDir() - // Handle "misc" category: real skill dir is skills/, not skills/misc/ + // Handle 'misc' category: real skill dir is skills/, not skills/misc/ let realPath = filePath if (filePath.startsWith('misc/')) { realPath = filePath.slice(5) @@ -307,8 +362,28 @@ export async function readFile_(ctx: any) { ctx.body = { error: 'Access denied' } return } - const content = await safeReadFile(fullPath) + let content = await safeReadFile(fullPath) if (content === null) { + // Fallback: recursive search for nested skills (e.g., mlops/lm-evaluation-harness/SKILL.md + // where actual path is mlops/evaluation/lm-evaluation-harness/SKILL.md) + const parts = filePath.split('/') + if (parts.length >= 2) { + const category = parts[0] + const skillName = parts[1] + const restPath = parts.slice(2).join('/') + const catDir = join(hd, 'skills', category) + const skillDir = await findSkillDirByName(catDir, skillName) + if (skillDir) { + const resolvedPath = resolve(join(skillDir, restPath)) + if (isPathWithin(resolvedPath, skillDir)) { + const nestedContent = await safeReadFile(resolvedPath) + if (nestedContent !== null) { + ctx.body = { content: nestedContent } + return + } + } + } + } ctx.status = 404 ctx.body = { error: 'File not found' } return