From d9b509ae89ba65aa247127bf91894bbc8cbae567 Mon Sep 17 00:00:00 2001 From: terra tauri Date: Fri, 20 Feb 2026 01:02:00 -0800 Subject: [PATCH] fix: handle nested hooks.json, skip frontmatterless markdown, fix error display mergeHooks() crashed on hooks.json files with a wrapper object (e.g., { description: "...", hooks: { EventName: [...] } }). Now detects nested format and skips non-array values. classifyComponent() now requires frontmatter for heuristic classification (steps 4+). Markdown files without frontmatter that don't match by filename or location are silently skipped instead of producing classification errors. This fixes reference docs and planning notes being flagged. action.yml error display was dead code due to set -e killing the shell before the $? check. Replaced with if-! pattern. Co-Authored-By: Claude Opus 4.6 --- agentic-marketplace/discover/action.yml | 4 +- scripts/dist/discover-components.cjs | 13 +- scripts/src/discover-components.js | 31 ++-- scripts/test/discover-components.test.js | 171 ++++++++++++++++++++++- 4 files changed, 203 insertions(+), 16 deletions(-) diff --git a/agentic-marketplace/discover/action.yml b/agentic-marketplace/discover/action.yml index 1ef71bf..d114618 100644 --- a/agentic-marketplace/discover/action.yml +++ b/agentic-marketplace/discover/action.yml @@ -36,9 +36,7 @@ runs: fi echo "Discovering components..." - OUTPUT=$(node "$SCRIPT_PATH" discover-all 2>&1) - - if [ $? -ne 0 ]; then + if ! OUTPUT=$(node "$SCRIPT_PATH" discover-all 2>&1); then echo "ERROR: Discovery failed" >&2 echo "$OUTPUT" >&2 exit 1 diff --git a/scripts/dist/discover-components.cjs b/scripts/dist/discover-components.cjs index d042bcc..7993510 100755 --- a/scripts/dist/discover-components.cjs +++ b/scripts/dist/discover-components.cjs @@ -7382,6 +7382,9 @@ function classifyComponent(filePath, rootDir, config) { if (pathParts[0] === "skills" || pathParts.includes("skills")) { return { type: "skill", path: path.dirname(filePath) }; } + if (!frontmatter || Object.keys(frontmatter).length === 0) { + return { type: null, path: filePath, skipped: true }; + } if (frontmatter) { const hasExamples = Array.isArray(frontmatter.examples); const hasVersion = typeof frontmatter.version === "string"; @@ -7464,7 +7467,7 @@ function discoverMarkdownComponents(rootDir, config) { commands.push(classified.path); } else if (classified.type === "agent") { agents.push(classified.path); - } else { + } else if (!classified.skipped) { errors.push({ path: fullPath, error: classified.error }); } } @@ -7573,14 +7576,18 @@ function mergeHooks(hooksFiles) { return null; const merged = {}; for (const { content } of hooksFiles) { - for (const [event, hooks] of Object.entries(content)) { + const hooksMap = content.hooks && typeof content.hooks === "object" && !Array.isArray(content.hooks) ? content.hooks : content; + for (const [event, hooks] of Object.entries(hooksMap)) { + if (!Array.isArray(hooks)) { + continue; + } if (!merged[event]) { merged[event] = []; } merged[event].push(...hooks); } } - return merged; + return Object.keys(merged).length > 0 ? merged : null; } function mergeMcpServers(mcpFiles) { if (mcpFiles.length === 0) diff --git a/scripts/src/discover-components.js b/scripts/src/discover-components.js index 6cb8012..560d316 100644 --- a/scripts/src/discover-components.js +++ b/scripts/src/discover-components.js @@ -236,7 +236,14 @@ function classifyComponent(filePath, rootDir, config) { return { type: 'skill', path: path.dirname(filePath) }; } - // 4. FIELD HEURISTICS + // 4. REQUIRE FRONTMATTER for remaining heuristics + // Files without frontmatter that didn't match by name or location + // are not components (e.g., reference docs, planning notes) + if (!frontmatter || Object.keys(frontmatter).length === 0) { + return { type: null, path: filePath, skipped: true }; + } + + // 5. FIELD HEURISTICS if (frontmatter) { const hasExamples = Array.isArray(frontmatter.examples); const hasVersion = typeof frontmatter.version === 'string'; @@ -252,7 +259,7 @@ function classifyComponent(filePath, rootDir, config) { } } - // 5. DIRECTORY STRUCTURE + // 6. DIRECTORY STRUCTURE const dirName = path.dirname(filePath); const baseName = path.basename(filePath, '.md'); @@ -261,7 +268,7 @@ function classifyComponent(filePath, rootDir, config) { return { type: 'skill', path: dirName }; } - // 6. UNCLASSIFIED → ERROR + // 7. UNCLASSIFIED → ERROR const error = `Unable to classify component at '${relPath}'\n\n` + `To fix, add one of the following to your frontmatter:\n` + ` type: skill # For instructional content with supporting files\n` + @@ -347,8 +354,8 @@ function discoverMarkdownComponents(rootDir, config) { commands.push(classified.path); } else if (classified.type === 'agent') { agents.push(classified.path); - } else { - // Unclassified component - add to errors + } else if (!classified.skipped) { + // Unclassified component with frontmatter - add to errors errors.push({ path: fullPath, error: classified.error }); } } @@ -484,12 +491,18 @@ function discoverMcpFiles(rootDir, config) { function mergeHooks(hooksFiles) { if (hooksFiles.length === 0) return null; - // For now, just merge all hook arrays - // In the future, we could check for conflicts const merged = {}; for (const { content } of hooksFiles) { - for (const [event, hooks] of Object.entries(content)) { + // Support nested format: { description: "...", hooks: { EventName: [...] } } + const hooksMap = (content.hooks && typeof content.hooks === 'object' && !Array.isArray(content.hooks)) + ? content.hooks + : content; + + for (const [event, hooks] of Object.entries(hooksMap)) { + if (!Array.isArray(hooks)) { + continue; + } if (!merged[event]) { merged[event] = []; } @@ -497,7 +510,7 @@ function mergeHooks(hooksFiles) { } } - return merged; + return Object.keys(merged).length > 0 ? merged : null; } /** diff --git a/scripts/test/discover-components.test.js b/scripts/test/discover-components.test.js index 8334b25..ad6f01f 100644 --- a/scripts/test/discover-components.test.js +++ b/scripts/test/discover-components.test.js @@ -4,16 +4,20 @@ * Run: node scripts/test/discover-components.test.js */ +const fs = require('fs'); +const os = require('os'); const path = require('path'); const assert = require('assert'); const { loadConfig, + classifyComponent, discoverAllComponents, getCategoryNames, groupIntoPlugins, discoverPlugins, - validateSkill + validateSkill, + mergeHooks } = require('../src/discover-components.js'); const FIXTURES_DIR = path.resolve(__dirname, '../../test-fixtures/valid'); @@ -223,6 +227,171 @@ test('standalone skill validates successfully', () => { assert.strictEqual(result.name, 'standalone-skill'); }); +// --- mergeHooks --- + +console.log('\nmergeHooks'); + +test('handles nested hooks.json format with description + hooks wrapper', () => { + const hooksFiles = [{ + path: '/fake/hooks/hooks.json', + content: { + description: 'Design engineering hooks', + hooks: { + UserPromptSubmit: [{ type: 'command', command: 'echo hello' }] + } + } + }]; + const result = mergeHooks(hooksFiles); + assert.deepStrictEqual(result, { + UserPromptSubmit: [{ type: 'command', command: 'echo hello' }] + }); +}); + +test('handles flat hooks.json format (backward compatibility)', () => { + const hooksFiles = [{ + path: '/fake/hooks/hooks.json', + content: { + PreToolUse: [{ type: 'command', command: 'echo pre' }] + } + }]; + const result = mergeHooks(hooksFiles); + assert.deepStrictEqual(result, { + PreToolUse: [{ type: 'command', command: 'echo pre' }] + }); +}); + +test('merges mixed nested and flat formats from multiple files', () => { + const hooksFiles = [ + { + path: '/fake/a/hooks.json', + content: { + description: 'Plugin A hooks', + hooks: { + UserPromptSubmit: [{ type: 'command', command: 'echo a' }] + } + } + }, + { + path: '/fake/b/hooks.json', + content: { + UserPromptSubmit: [{ type: 'command', command: 'echo b' }], + PreToolUse: [{ type: 'command', command: 'echo pre' }] + } + } + ]; + const result = mergeHooks(hooksFiles); + assert.strictEqual(result.UserPromptSubmit.length, 2); + assert.strictEqual(result.PreToolUse.length, 1); +}); + +test('skips non-array values gracefully', () => { + const hooksFiles = [{ + path: '/fake/hooks/hooks.json', + content: { + description: 'Some description', + version: '1.0', + hooks: { + UserPromptSubmit: [{ type: 'command', command: 'echo hello' }] + } + } + }]; + const result = mergeHooks(hooksFiles); + // Should only contain UserPromptSubmit, not description/version + assert.deepStrictEqual(Object.keys(result), ['UserPromptSubmit']); +}); + +test('returns null when no arrays found', () => { + const hooksFiles = [{ + path: '/fake/hooks/hooks.json', + content: { description: 'Just metadata', version: '1.0' } + }]; + const result = mergeHooks(hooksFiles); + assert.strictEqual(result, null); +}); + +// --- classifyComponent --- + +console.log('\nclassifyComponent'); + +// Helper to create temp files for classifyComponent tests +function withTempDir(fn) { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'discover-test-')); + try { + fn(tmpDir); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } +} + +const defaultConfig = { + discovery: { + skillFilename: 'SKILL.md', + commandsDir: 'commands', + agentsDir: 'agents' + } +}; + +test('.md file with no frontmatter is skipped (not an error)', () => { + withTempDir((tmpDir) => { + const filePath = path.join(tmpDir, 'notes.md'); + fs.writeFileSync(filePath, '# Just some notes\n\nNo frontmatter here.\n'); + const result = classifyComponent(filePath, tmpDir, defaultConfig); + assert.strictEqual(result.type, null); + assert.strictEqual(result.skipped, true); + assert.strictEqual(result.error, undefined); + }); +}); + +test('.md file with frontmatter but no type uses heuristics', () => { + withTempDir((tmpDir) => { + const filePath = path.join(tmpDir, 'something.md'); + fs.writeFileSync(filePath, '---\nexamples:\n - test\n---\nContent\n'); + const result = classifyComponent(filePath, tmpDir, defaultConfig); + // Should use field heuristics: has examples array without version → command + assert.strictEqual(result.type, 'command'); + }); +}); + +test('SKILL.md with no frontmatter is classified as skill (filename match)', () => { + withTempDir((tmpDir) => { + const filePath = path.join(tmpDir, 'SKILL.md'); + fs.writeFileSync(filePath, '# My Skill\n\nNo frontmatter.\n'); + const result = classifyComponent(filePath, tmpDir, defaultConfig); + assert.strictEqual(result.type, 'skill'); + }); +}); + +test('.md in commands/ dir with no frontmatter is classified as command (location match)', () => { + withTempDir((tmpDir) => { + const cmdDir = path.join(tmpDir, 'commands'); + fs.mkdirSync(cmdDir); + const filePath = path.join(cmdDir, 'do-thing.md'); + fs.writeFileSync(filePath, '# Do the thing\n\nNo frontmatter.\n'); + const result = classifyComponent(filePath, tmpDir, defaultConfig); + assert.strictEqual(result.type, 'command'); + }); +}); + +test('.md in agents/ dir with no frontmatter is classified as agent (location match)', () => { + withTempDir((tmpDir) => { + const agentDir = path.join(tmpDir, 'agents'); + fs.mkdirSync(agentDir); + const filePath = path.join(agentDir, 'helper.md'); + fs.writeFileSync(filePath, '# Helper agent\n\nNo frontmatter.\n'); + const result = classifyComponent(filePath, tmpDir, defaultConfig); + assert.strictEqual(result.type, 'agent'); + }); +}); + +test('.md with explicit type in frontmatter takes priority', () => { + withTempDir((tmpDir) => { + const filePath = path.join(tmpDir, 'random.md'); + fs.writeFileSync(filePath, '---\ntype: agent\nname: test\n---\nContent\n'); + const result = classifyComponent(filePath, tmpDir, defaultConfig); + assert.strictEqual(result.type, 'agent'); + }); +}); + // --- Summary --- console.log(`\n${passed + failed} tests: ${passed} passed, ${failed} failed`);