From 770505191003c21bab81272931e17654b4de3a15 Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Tue, 10 Mar 2026 19:31:02 -0700 Subject: [PATCH] fix: align architecture tooling with current hooks docs --- AGENTS.md | 2 +- package.json | 2 +- schemas/hooks.schema.json | 131 +++++++++++++++++++++++++++++++---- scripts/ci/catalog.js | 44 +++++++----- scripts/ci/validate-hooks.js | 115 ++++++++++++++++++++++++------ tests/ci/validators.test.js | 29 +++++++- tests/hooks/hooks.test.js | 1 + tests/run-all.js | 11 +++ 8 files changed, 282 insertions(+), 53 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3c0ada3b..8e9e9fd1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -130,7 +130,7 @@ Troubleshoot failures: check test isolation → verify mocks → fix implementat ``` agents/ — 13 specialized subagents skills/ — 65+ workflow skills and domain knowledge -commands/ — 33 slash commands +commands/ — 40 slash commands hooks/ — Trigger-based automations rules/ — Always-follow guidelines (common + per-language) scripts/ — Cross-platform Node.js utilities diff --git a/package.json b/package.json index 65054448..1acb0444 100644 --- a/package.json +++ b/package.json @@ -84,7 +84,7 @@ "lint": "eslint . && markdownlint '**/*.md' --ignore node_modules", "claw": "node scripts/claw.js", "test": "node scripts/ci/validate-agents.js && node scripts/ci/validate-commands.js && node scripts/ci/validate-rules.js && node scripts/ci/validate-skills.js && node scripts/ci/validate-hooks.js && node scripts/ci/validate-no-personal-paths.js && node tests/run-all.js", - "coverage": "c8 --all --include='scripts/**/*.js' --reporter=text --reporter=lcov node tests/run-all.js" + "coverage": "c8 --all --include=\"scripts/**/*.js\" --check-coverage --lines 80 --functions 80 --branches 80 --statements 80 --reporter=text --reporter=lcov node tests/run-all.js" }, "devDependencies": { "@eslint/js": "^9.39.2", diff --git a/schemas/hooks.schema.json b/schemas/hooks.schema.json index 3aa2ba1f..4d119297 100644 --- a/schemas/hooks.schema.json +++ b/schemas/hooks.schema.json @@ -1,9 +1,17 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "title": "Claude Code Hooks Configuration", - "description": "Configuration for Claude Code hooks. Event types are validated at runtime and must be one of: PreToolUse, PostToolUse, PreCompact, SessionStart, SessionEnd, Stop, Notification, SubagentStop", + "description": "Configuration for Claude Code hooks. Supports current Claude Code hook events and hook action types.", "$defs": { - "hookItem": { + "stringArray": { + "type": "array", + "items": { + "type": "string", + "minLength": 1 + }, + "minItems": 1 + }, + "commandHookItem": { "type": "object", "required": [ "type", @@ -12,19 +20,17 @@ "properties": { "type": { "type": "string", - "enum": ["command", "notification"], - "description": "Hook action type (command or notification)" + "const": "command", + "description": "Run a local command" }, "command": { "oneOf": [ { - "type": "string" + "type": "string", + "minLength": 1 }, { - "type": "array", - "items": { - "type": "string" - } + "$ref": "#/$defs/stringArray" } ] }, @@ -37,17 +43,94 @@ "minimum": 0, "description": "Timeout in seconds for async hooks" } - } + }, + "additionalProperties": true + }, + "httpHookItem": { + "type": "object", + "required": [ + "type", + "url" + ], + "properties": { + "type": { + "type": "string", + "const": "http" + }, + "url": { + "type": "string", + "minLength": 1 + }, + "headers": { + "type": "object", + "additionalProperties": { + "type": "string" + } + }, + "allowedEnvVars": { + "$ref": "#/$defs/stringArray" + }, + "timeout": { + "type": "number", + "minimum": 0 + } + }, + "additionalProperties": true + }, + "promptHookItem": { + "type": "object", + "required": [ + "type", + "prompt" + ], + "properties": { + "type": { + "type": "string", + "enum": ["prompt", "agent"] + }, + "prompt": { + "type": "string", + "minLength": 1 + }, + "model": { + "type": "string", + "minLength": 1 + }, + "timeout": { + "type": "number", + "minimum": 0 + } + }, + "additionalProperties": true + }, + "hookItem": { + "oneOf": [ + { + "$ref": "#/$defs/commandHookItem" + }, + { + "$ref": "#/$defs/httpHookItem" + }, + { + "$ref": "#/$defs/promptHookItem" + } + ] }, "matcherEntry": { "type": "object", "required": [ - "matcher", "hooks" ], "properties": { "matcher": { - "type": "string" + "oneOf": [ + { + "type": "string" + }, + { + "type": "object" + } + ] }, "hooks": { "type": "array", @@ -70,6 +153,28 @@ }, "hooks": { "type": "object", + "propertyNames": { + "enum": [ + "SessionStart", + "UserPromptSubmit", + "PreToolUse", + "PermissionRequest", + "PostToolUse", + "PostToolUseFailure", + "Notification", + "SubagentStart", + "Stop", + "SubagentStop", + "PreCompact", + "InstructionsLoaded", + "TeammateIdle", + "TaskCompleted", + "ConfigChange", + "WorktreeCreate", + "WorktreeRemove", + "SessionEnd" + ] + }, "additionalProperties": { "type": "array", "items": { @@ -89,4 +194,4 @@ } } ] -} \ No newline at end of file +} diff --git a/scripts/ci/catalog.js b/scripts/ci/catalog.js index 1e279445..02a71e78 100644 --- a/scripts/ci/catalog.js +++ b/scripts/ci/catalog.js @@ -17,27 +17,39 @@ const SKILLS_DIR = path.join(ROOT, 'skills'); function listAgents() { if (!fs.existsSync(AGENTS_DIR)) return []; - return fs.readdirSync(AGENTS_DIR) - .filter(f => f.endsWith('.md')) - .map(f => f.slice(0, -3)) - .sort(); + try { + return fs.readdirSync(AGENTS_DIR) + .filter(f => f.endsWith('.md')) + .map(f => f.slice(0, -3)) + .sort(); + } catch (error) { + throw new Error(`Failed to read agents directory (${AGENTS_DIR}): ${error.message}`); + } } function listCommands() { if (!fs.existsSync(COMMANDS_DIR)) return []; - return fs.readdirSync(COMMANDS_DIR) - .filter(f => f.endsWith('.md')) - .map(f => f.slice(0, -3)) - .sort(); + try { + return fs.readdirSync(COMMANDS_DIR) + .filter(f => f.endsWith('.md')) + .map(f => f.slice(0, -3)) + .sort(); + } catch (error) { + throw new Error(`Failed to read commands directory (${COMMANDS_DIR}): ${error.message}`); + } } function listSkills() { if (!fs.existsSync(SKILLS_DIR)) return []; - const entries = fs.readdirSync(SKILLS_DIR, { withFileTypes: true }); - return entries - .filter(e => e.isDirectory() && fs.existsSync(path.join(SKILLS_DIR, e.name, 'SKILL.md'))) - .map(e => e.name) - .sort(); + try { + const entries = fs.readdirSync(SKILLS_DIR, { withFileTypes: true }); + return entries + .filter(e => e.isDirectory() && fs.existsSync(path.join(SKILLS_DIR, e.name, 'SKILL.md'))) + .map(e => e.name) + .sort(); + } catch (error) { + throw new Error(`Failed to read skills directory (${SKILLS_DIR}): ${error.message}`); + } } function run() { @@ -58,11 +70,11 @@ function run() { console.log(`- **Commands:** ${catalog.commands.count}`); console.log(`- **Skills:** ${catalog.skills.count}\n`); console.log('## Agents\n'); - catalog.agents.list.forEach(a => console.log(`- ${a}`)); + catalog.agents.list.forEach(a => { console.log(`- ${a}`); }); console.log('\n## Commands\n'); - catalog.commands.list.forEach(c => console.log(`- ${c}`)); + catalog.commands.list.forEach(c => { console.log(`- ${c}`); }); console.log('\n## Skills\n'); - catalog.skills.list.forEach(s => console.log(`- ${s}`)); + catalog.skills.list.forEach(s => { console.log(`- ${s}`); }); } else { console.log(JSON.stringify(catalog, null, 2)); } diff --git a/scripts/ci/validate-hooks.js b/scripts/ci/validate-hooks.js index 7dd5ebee..b4e440d9 100644 --- a/scripts/ci/validate-hooks.js +++ b/scripts/ci/validate-hooks.js @@ -10,7 +10,36 @@ const Ajv = require('ajv'); const HOOKS_FILE = path.join(__dirname, '../../hooks/hooks.json'); const HOOKS_SCHEMA_PATH = path.join(__dirname, '../../schemas/hooks.schema.json'); -const VALID_EVENTS = ['PreToolUse', 'PostToolUse', 'PreCompact', 'SessionStart', 'SessionEnd', 'Stop', 'Notification', 'SubagentStop']; +const VALID_EVENTS = [ + 'SessionStart', + 'UserPromptSubmit', + 'PreToolUse', + 'PermissionRequest', + 'PostToolUse', + 'PostToolUseFailure', + 'Notification', + 'SubagentStart', + 'Stop', + 'SubagentStop', + 'PreCompact', + 'InstructionsLoaded', + 'TeammateIdle', + 'TaskCompleted', + 'ConfigChange', + 'WorktreeCreate', + 'WorktreeRemove', + 'SessionEnd', +]; +const VALID_HOOK_TYPES = ['command', 'http', 'prompt', 'agent']; +const EVENTS_WITHOUT_MATCHER = new Set(['UserPromptSubmit', 'Notification', 'Stop', 'SubagentStop']); + +function isNonEmptyString(value) { + return typeof value === 'string' && value.trim().length > 0; +} + +function isNonEmptyStringArray(value) { + return Array.isArray(value) && value.length > 0 && value.every(item => isNonEmptyString(item)); +} /** * Validate a single hook entry has required fields and valid inline JS @@ -24,32 +53,72 @@ function validateHookEntry(hook, label) { if (!hook.type || typeof hook.type !== 'string') { console.error(`ERROR: ${label} missing or invalid 'type' field`); hasErrors = true; - } - - // Validate optional async and timeout fields - if ('async' in hook && typeof hook.async !== 'boolean') { - console.error(`ERROR: ${label} 'async' must be a boolean`); + } else if (!VALID_HOOK_TYPES.includes(hook.type)) { + console.error(`ERROR: ${label} has unsupported hook type '${hook.type}'`); hasErrors = true; } + if ('timeout' in hook && (typeof hook.timeout !== 'number' || hook.timeout < 0)) { console.error(`ERROR: ${label} 'timeout' must be a non-negative number`); hasErrors = true; } - if (!hook.command || (typeof hook.command !== 'string' && !Array.isArray(hook.command)) || (typeof hook.command === 'string' && !hook.command.trim()) || (Array.isArray(hook.command) && (hook.command.length === 0 || !hook.command.every(s => typeof s === 'string' && s.length > 0)))) { - console.error(`ERROR: ${label} missing or invalid 'command' field`); - hasErrors = true; - } else if (typeof hook.command === 'string') { - // Validate inline JS syntax in node -e commands - const nodeEMatch = hook.command.match(/^node -e "(.*)"$/s); - if (nodeEMatch) { - try { - new vm.Script(nodeEMatch[1].replace(/\\\\/g, '\\').replace(/\\"/g, '"').replace(/\\n/g, '\n').replace(/\\t/g, '\t')); - } catch (syntaxErr) { - console.error(`ERROR: ${label} has invalid inline JS: ${syntaxErr.message}`); - hasErrors = true; + if (hook.type === 'command') { + if ('async' in hook && typeof hook.async !== 'boolean') { + console.error(`ERROR: ${label} 'async' must be a boolean`); + hasErrors = true; + } + + if (!isNonEmptyString(hook.command) && !isNonEmptyStringArray(hook.command)) { + console.error(`ERROR: ${label} missing or invalid 'command' field`); + hasErrors = true; + } else if (typeof hook.command === 'string') { + const nodeEMatch = hook.command.match(/^node -e "(.*)"$/s); + if (nodeEMatch) { + try { + new vm.Script(nodeEMatch[1].replace(/\\\\/g, '\\').replace(/\\"/g, '"').replace(/\\n/g, '\n').replace(/\\t/g, '\t')); + } catch (syntaxErr) { + console.error(`ERROR: ${label} has invalid inline JS: ${syntaxErr.message}`); + hasErrors = true; + } } } + + return hasErrors; + } + + if ('async' in hook) { + console.error(`ERROR: ${label} 'async' is only supported for command hooks`); + hasErrors = true; + } + + if (hook.type === 'http') { + if (!isNonEmptyString(hook.url)) { + console.error(`ERROR: ${label} missing or invalid 'url' field`); + hasErrors = true; + } + + if ('headers' in hook && (typeof hook.headers !== 'object' || hook.headers === null || Array.isArray(hook.headers) || !Object.values(hook.headers).every(value => typeof value === 'string'))) { + console.error(`ERROR: ${label} 'headers' must be an object with string values`); + hasErrors = true; + } + + if ('allowedEnvVars' in hook && (!Array.isArray(hook.allowedEnvVars) || !hook.allowedEnvVars.every(value => isNonEmptyString(value)))) { + console.error(`ERROR: ${label} 'allowedEnvVars' must be an array of strings`); + hasErrors = true; + } + + return hasErrors; + } + + if (!isNonEmptyString(hook.prompt)) { + console.error(`ERROR: ${label} missing or invalid 'prompt' field`); + hasErrors = true; + } + + if ('model' in hook && !isNonEmptyString(hook.model)) { + console.error(`ERROR: ${label} 'model' must be a non-empty string`); + hasErrors = true; } return hasErrors; @@ -110,9 +179,12 @@ function validateHooks() { hasErrors = true; continue; } - if (!matcher.matcher) { + if (!('matcher' in matcher) && !EVENTS_WITHOUT_MATCHER.has(eventType)) { console.error(`ERROR: ${eventType}[${i}] missing 'matcher' field`); hasErrors = true; + } else if ('matcher' in matcher && typeof matcher.matcher !== 'string' && (typeof matcher.matcher !== 'object' || matcher.matcher === null)) { + console.error(`ERROR: ${eventType}[${i}] has invalid 'matcher' field`); + hasErrors = true; } if (!matcher.hooks || !Array.isArray(matcher.hooks)) { console.error(`ERROR: ${eventType}[${i}] missing 'hooks' array`); @@ -132,9 +204,12 @@ function validateHooks() { // Array format (legacy) for (let i = 0; i < hooks.length; i++) { const hook = hooks[i]; - if (!hook.matcher) { + if (!('matcher' in hook)) { console.error(`ERROR: Hook ${i} missing 'matcher' field`); hasErrors = true; + } else if (typeof hook.matcher !== 'string' && (typeof hook.matcher !== 'object' || hook.matcher === null)) { + console.error(`ERROR: Hook ${i} has invalid 'matcher' field`); + hasErrors = true; } if (!hook.hooks || !Array.isArray(hook.hooks)) { console.error(`ERROR: Hook ${i} missing 'hooks' array`); diff --git a/tests/ci/validators.test.js b/tests/ci/validators.test.js index 6077a389..f016a538 100644 --- a/tests/ci/validators.test.js +++ b/tests/ci/validators.test.js @@ -1927,7 +1927,7 @@ function runTests() { PreToolUse: [{ matcher: 'Write', hooks: [{ - type: 'intercept', + type: 'command', command: 'echo test', async: 'yes' // Should be boolean, not string }] @@ -1947,7 +1947,7 @@ function runTests() { PostToolUse: [{ matcher: 'Edit', hooks: [{ - type: 'intercept', + type: 'command', command: 'echo test', timeout: -5 // Must be non-negative }] @@ -2105,6 +2105,31 @@ function runTests() { cleanupTestDir(testDir); })) passed++; else failed++; + console.log('\nRound 82b: validate-hooks (current official events and hook types):'); + + if (test('accepts UserPromptSubmit with omitted matcher and prompt/http/agent hooks', () => { + const testDir = createTestDir(); + const hooksJson = JSON.stringify({ + hooks: { + UserPromptSubmit: [ + { + hooks: [ + { type: 'prompt', prompt: 'Summarize the request.' }, + { type: 'agent', prompt: 'Review for security issues.', model: 'gpt-5.4' }, + { type: 'http', url: 'https://example.com/hooks', headers: { Authorization: 'Bearer token' } } + ] + } + ] + } + }); + const hooksFile = path.join(testDir, 'hooks.json'); + fs.writeFileSync(hooksFile, hooksJson); + + const result = runValidatorWithDir('validate-hooks', 'HOOKS_FILE', hooksFile); + assert.strictEqual(result.code, 0, 'Should accept current official hook event/type combinations'); + cleanupTestDir(testDir); + })) passed++; else failed++; + // ── Round 83: validate-agents whitespace-only field, validate-skills empty SKILL.md ── console.log('\nRound 83: validate-agents (whitespace-only frontmatter field value):'); diff --git a/tests/hooks/hooks.test.js b/tests/hooks/hooks.test.js index e13e8193..5610ed37 100644 --- a/tests/hooks/hooks.test.js +++ b/tests/hooks/hooks.test.js @@ -2561,6 +2561,7 @@ async function runTests() { assert.ok(!runAllSource.includes('execSync'), 'Should not use execSync'); // Verify it shows stderr assert.ok(runAllSource.includes('stderr'), 'Should handle stderr output'); + assert.ok(runAllSource.includes('result.status !== 0'), 'Should treat non-zero child exits as failures'); })) passed++; else failed++; // ── Round 32: post-edit-typecheck special characters & check-console-log ── diff --git a/tests/run-all.js b/tests/run-all.js index 25d44666..399de5a7 100644 --- a/tests/run-all.js +++ b/tests/run-all.js @@ -71,6 +71,17 @@ for (const testFile of testFiles) { if (passedMatch) totalPassed += parseInt(passedMatch[1], 10); if (failedMatch) totalFailed += parseInt(failedMatch[1], 10); + + if (result.error) { + console.log(`✗ ${testFile} failed to start: ${result.error.message}`); + totalFailed += failedMatch ? 0 : 1; + continue; + } + + if (result.status !== 0) { + console.log(`✗ ${testFile} exited with status ${result.status}`); + totalFailed += failedMatch ? 0 : 1; + } } totalTests = totalPassed + totalFailed;