From 3691db80d718fb46ff5ee6431352728df1dc0df6 Mon Sep 17 00:00:00 2001 From: Flavio Tavares Date: Mon, 10 Aug 2026 21:08:22 +0100 Subject: [PATCH 1/2] fix(doctor): resolve settings-json and hooks-claude-count false positives Both checks warned on a healthy framework checkout: - settings-json required >= 40 deny rules unconditionally, ignoring boundary.frameworkProtection. Contributors set it to false precisely so L1/L2 paths stay editable, making an empty deny list correct in that mode. Now reads the flag and PASSes when protection is off; defaults to protected when the config or key is absent. - hooks-claude-count only inspected settings.local.json, but this repo registers its hooks in settings.json, which Claude Code merges equally. Now collects commands from both files and reports how many of the discovered .cjs files are referenced, since engine hooks are spawned by their wrappers rather than registered directly. aiox doctor goes from 16 PASS / 2 WARN to 18 PASS / 0 WARN with no behaviour change for project installs. Tests: 8 new cases (59 total). Verified 5 of them fail against the unpatched checks; the other 3 are regression guards for the protected-by-default, explicit-true, and malformed-JSON paths. Co-Authored-By: Claude Opus 5 (1M context) --- .../core/doctor/checks/hooks-claude-count.js | 89 +++++---- .../core/doctor/checks/settings-json.js | 46 +++++ .aiox-core/install-manifest.yaml | 10 +- .../tests/unit/doctor/doctor-checks.test.js | 173 ++++++++++++++++++ 4 files changed, 277 insertions(+), 41 deletions(-) diff --git a/.aiox-core/core/doctor/checks/hooks-claude-count.js b/.aiox-core/core/doctor/checks/hooks-claude-count.js index 7b4ff02165..0c7737f008 100644 --- a/.aiox-core/core/doctor/checks/hooks-claude-count.js +++ b/.aiox-core/core/doctor/checks/hooks-claude-count.js @@ -15,6 +15,45 @@ const fs = require('fs'); const name = 'hooks-claude-count'; +/** + * Extracts every hook command string from a Claude Code settings file. + * + * Hooks may be registered in settings.json (shipped/tracked) or + * settings.local.json (per-machine); Claude Code merges both, so + * registration in either counts. + * + * Returns [] when the file is missing or unparseable. + */ +function collectHookCommands(settingsPath) { + if (!fs.existsSync(settingsPath)) return []; + + let settings; + try { + settings = JSON.parse(fs.readFileSync(settingsPath, 'utf8')); + } catch { + return []; + } + + const hooks = settings.hooks || {}; + // Claude Code hooks schema: { EventName: [{ matcher, hooks: [{ type, command }] }] } + const commands = []; + for (const entries of Object.values(hooks)) { + if (!Array.isArray(entries)) continue; + for (const entry of entries) { + if (entry && Array.isArray(entry.hooks)) { + for (const h of entry.hooks) { + if (h && h.command) commands.push(h.command); + } + } + // Fallback: flat string or direct command + if (typeof entry === 'string') commands.push(entry); + if (entry && typeof entry.command === 'string') commands.push(entry.command); + } + } + + return commands; +} + async function run(context) { const hooksDir = path.join(context.projectRoot, '.claude', 'hooks'); @@ -53,47 +92,25 @@ async function run(context) { }; } - // Check registration in settings.local.json - const settingsLocalPath = path.join(context.projectRoot, '.claude', 'settings.local.json'); - let registered = false; - - if (fs.existsSync(settingsLocalPath)) { - try { - const settingsLocal = JSON.parse(fs.readFileSync(settingsLocalPath, 'utf8')); - const hooks = settingsLocal.hooks || {}; - // Claude Code hooks schema: { EventName: [{ matcher, hooks: [{ type, command }] }] } - const allHookCommands = []; - for (const entries of Object.values(hooks)) { - if (!Array.isArray(entries)) continue; - for (const entry of entries) { - if (entry && Array.isArray(entry.hooks)) { - for (const h of entry.hooks) { - if (h && h.command) allHookCommands.push(h.command); - } - } - // Fallback: flat string or direct command - if (typeof entry === 'string') allHookCommands.push(entry); - if (entry && typeof entry.command === 'string') allHookCommands.push(entry.command); - } - } - const hooksStr = allHookCommands.join('\n'); - - // Check if at least some hook files are referenced in settings - const referencedCount = hookFiles.filter( - (f) => hooksStr.includes(f.name) || hooksStr.includes(f.name.replace('.cjs', '')), - ).length; + // Check registration in settings.json and settings.local.json (Claude Code merges both). + // Wrapper hooks are registered directly; engine hooks they spawn as child + // processes are not, so any reference is enough to count as wired up. + const claudeDir = path.join(context.projectRoot, '.claude'); + const hooksStr = [ + ...collectHookCommands(path.join(claudeDir, 'settings.json')), + ...collectHookCommands(path.join(claudeDir, 'settings.local.json')), + ].join('\n'); - registered = referencedCount > 0; - } catch { - registered = false; - } - } + const referencedCount = hookFiles.filter( + (f) => hooksStr.includes(f.name) || hooksStr.includes(f.name.replace('.cjs', '')), + ).length; + const registered = referencedCount > 0; if (hookCount >= 2 && registered) { return { check: name, status: 'PASS', - message: `${hookCount} hook files found and registered`, + message: `${hookCount} hook files found, ${referencedCount} registered`, fixCommand: null, }; } @@ -102,7 +119,7 @@ async function run(context) { return { check: name, status: 'WARN', - message: `${hookCount} hook files found but not registered in settings.local.json`, + message: `${hookCount} hook files found but not registered in settings.json or settings.local.json`, fixCommand: 'npx aiox-core install --force', }; } diff --git a/.aiox-core/core/doctor/checks/settings-json.js b/.aiox-core/core/doctor/checks/settings-json.js index 535c7915b8..938e3c29ee 100644 --- a/.aiox-core/core/doctor/checks/settings-json.js +++ b/.aiox-core/core/doctor/checks/settings-json.js @@ -13,6 +13,42 @@ const fs = require('fs'); const name = 'settings-json'; +/** + * Reads boundary.frameworkProtection from core-config.yaml. + * + * Deny rules exist to stop project consumers from editing L1/L2 framework + * paths. Framework contributors set frameworkProtection: false precisely so + * those paths stay editable, so an empty deny list is correct in that mode. + * + * Defaults to true (protected) when the config or key is absent. + */ +function isFrameworkProtectionEnabled(context) { + const configPath = path.join(context.projectRoot, '.aiox-core', 'core-config.yaml'); + if (!fs.existsSync(configPath)) return true; + + let content; + try { + content = fs.readFileSync(configPath, 'utf8'); + } catch { + return true; + } + + let inBoundary = false; + for (const line of content.split('\n')) { + if (/^boundary:\s*$/.test(line)) { + inBoundary = true; + continue; + } + if (!inBoundary) continue; + // A new top-level key ends the boundary section + if (/^\S/.test(line)) break; + const match = line.match(/^\s+frameworkProtection:\s*(true|false)\b/); + if (match) return match[1] === 'true'; + } + + return true; +} + /** * Checks that core-config.yaml boundary.protected paths are covered by deny rules. * Returns array of unprotected boundary paths. @@ -90,6 +126,16 @@ async function run(context) { const denyCount = denyRules.length; const allowCount = allowRules.length; + // Contributor mode: boundary enforcement is off, so deny rules are not expected + if (!isFrameworkProtectionEnabled(context)) { + return { + check: name, + status: 'PASS', + message: `Deny rules not required (boundary.frameworkProtection: false — contributor mode, ${denyCount} rules, ${allowCount} allows)`, + fixCommand: null, + }; + } + if (denyCount < 40) { return { check: name, diff --git a/.aiox-core/install-manifest.yaml b/.aiox-core/install-manifest.yaml index b131eb1d2f..5cf09028bb 100644 --- a/.aiox-core/install-manifest.yaml +++ b/.aiox-core/install-manifest.yaml @@ -8,7 +8,7 @@ # - File types for categorization # version: 5.3.0 -generated_at: "2026-07-13T20:16:51.869Z" +generated_at: "2026-08-10T20:08:23.689Z" generator: scripts/generate-install-manifest.js file_count: 1165 files: @@ -337,9 +337,9 @@ files: type: core size: 1106 - path: core/doctor/checks/hooks-claude-count.js - hash: sha256:026ddf0248819b89b1147e0876a2934e38e0113d3c6380d68a752d432060e7ec + hash: sha256:94fd5136635b0eb766c0dfc7ccbcff19bc2d63dc5dea9fedd2e16f21ce609875 type: core - size: 3348 + size: 3824 - path: core/doctor/checks/ide-sync.js hash: sha256:4ddd037b4ad18c4201ca1428a1044efd313e9d2721cd399aebd3c5043fd4e2d1 type: core @@ -365,9 +365,9 @@ files: type: core size: 1368 - path: core/doctor/checks/settings-json.js - hash: sha256:bd26841b966fcfa003eca6f85416d4f877b9dcfea0e4017df9f2a97c14c33fbb + hash: sha256:ac981d682c7232a2fb0e2fe7920741bde45d1a2ce0d3a0bbe33c95b3a4cd8b31 type: core - size: 3286 + size: 4726 - path: core/doctor/checks/skills-count.js hash: sha256:811d904bde6d2ba4940f19cbe6a29cc12c5df6908ac95cb37bcb7add687fe4cc type: core diff --git a/packages/installer/tests/unit/doctor/doctor-checks.test.js b/packages/installer/tests/unit/doctor/doctor-checks.test.js index bca0e9bb07..123a5b8f9b 100644 --- a/packages/installer/tests/unit/doctor/doctor-checks.test.js +++ b/packages/installer/tests/unit/doctor/doctor-checks.test.js @@ -131,6 +131,95 @@ describe('settings-json check', () => { expect(result.status).toBe('WARN'); expect(result.message).toContain('boundary coverage'); }); + + it('should PASS with no deny rules when frameworkProtection is false', async () => { + fs.existsSync.mockReturnValue(true); + const mockSettings = { permissions: { deny: [], allow: [] } }; + const coreConfig = [ + 'boundary:', + ' frameworkProtection: false # TEMPORARY: contributor mode', + ' protected:', + ' - .aiox-core/core/**', + ].join('\n'); + fs.readFileSync.mockImplementation((p) => { + if (p.includes('settings.json')) return JSON.stringify(mockSettings); + if (p.includes('core-config')) return coreConfig; + return ''; + }); + + const result = await settingsJsonCheck.run(mockContext); + expect(result.status).toBe('PASS'); + expect(result.message).toContain('contributor mode'); + expect(result.fixCommand).toBeNull(); + }); + + it('should skip boundary coverage warning when frameworkProtection is false', async () => { + fs.existsSync.mockReturnValue(true); + const mockSettings = { + permissions: { + deny: new Array(50).fill('Edit(docs/)'), + allow: [], + }, + }; + const coreConfig = [ + 'boundary:', + ' frameworkProtection: false', + ' protected:', + ' - .aiox-core/core/**', + ' - bin/aiox.js', + ].join('\n'); + fs.readFileSync.mockImplementation((p) => { + if (p.includes('settings.json')) return JSON.stringify(mockSettings); + if (p.includes('core-config')) return coreConfig; + return ''; + }); + + const result = await settingsJsonCheck.run(mockContext); + expect(result.status).toBe('PASS'); + expect(result.message).not.toContain('boundary coverage'); + }); + + it('should still WARN when frameworkProtection is explicitly true', async () => { + fs.existsSync.mockReturnValue(true); + const mockSettings = { permissions: { deny: ['one'], allow: [] } }; + const coreConfig = [ + 'boundary:', + ' frameworkProtection: true', + ' protected:', + ' - .aiox-core/core/**', + ].join('\n'); + fs.readFileSync.mockImplementation((p) => { + if (p.includes('settings.json')) return JSON.stringify(mockSettings); + if (p.includes('core-config')) return coreConfig; + return ''; + }); + + const result = await settingsJsonCheck.run(mockContext); + expect(result.status).toBe('WARN'); + expect(result.message).toContain('below threshold'); + }); + + it('should default to protected when frameworkProtection key is absent', async () => { + fs.existsSync.mockReturnValue(true); + const mockSettings = { permissions: { deny: [], allow: [] } }; + // frameworkProtection lives under a later top-level key, not under boundary + const coreConfig = [ + 'boundary:', + ' protected:', + ' - .aiox-core/core/**', + 'other:', + ' frameworkProtection: false', + ].join('\n'); + fs.readFileSync.mockImplementation((p) => { + if (p.includes('settings.json')) return JSON.stringify(mockSettings); + if (p.includes('core-config')) return coreConfig; + return ''; + }); + + const result = await settingsJsonCheck.run(mockContext); + expect(result.status).toBe('WARN'); + expect(result.message).toContain('below threshold'); + }); }); describe('rules-files check', () => { @@ -613,6 +702,90 @@ describe('hooks-claude-count check', () => { const result = await hooksClaudeCountCheck.run(mockContext); expect(result.status).toBe('FAIL'); }); + + it('should PASS when registered in settings.json only', async () => { + fs.existsSync.mockReturnValue(true); + fs.readdirSync.mockReturnValue([ + fileEntry('synapse-wrapper.cjs'), + fileEntry('precompact-wrapper.cjs'), + ]); + const settings = { + hooks: { + UserPromptSubmit: [ + { hooks: [{ type: 'command', command: 'node .claude/hooks/synapse-wrapper.cjs' }] }, + ], + PreCompact: [ + { hooks: [{ type: 'command', command: 'node .claude/hooks/precompact-wrapper.cjs' }] }, + ], + }, + }; + fs.readFileSync.mockImplementation((p) => { + // settings.local.json carries permissions only — no hooks + if (p.includes('settings.local.json')) return JSON.stringify({ permissions: { allow: [] } }); + return JSON.stringify(settings); + }); + + const result = await hooksClaudeCountCheck.run(mockContext); + expect(result.status).toBe('PASS'); + expect(result.message).toContain('2 registered'); + }); + + it('should PASS when only wrapper hooks are registered and engines are spawned', async () => { + fs.existsSync.mockReturnValue(true); + fs.readdirSync.mockReturnValue([ + fileEntry('synapse-wrapper.cjs'), + fileEntry('synapse-engine.cjs'), + fileEntry('precompact-wrapper.cjs'), + fileEntry('precompact-session-digest.cjs'), + fileEntry('enforce-git-push-authority.cjs'), + ]); + const settings = { + hooks: { + UserPromptSubmit: [ + { hooks: [{ type: 'command', command: 'node .claude/hooks/synapse-wrapper.cjs' }] }, + ], + PreCompact: [ + { hooks: [{ type: 'command', command: 'node .claude/hooks/precompact-wrapper.cjs' }] }, + ], + PreToolUse: [ + { + matcher: 'Bash', + hooks: [ + { type: 'command', command: 'node .claude/hooks/enforce-git-push-authority.cjs' }, + ], + }, + ], + }, + }; + fs.readFileSync.mockImplementation((p) => { + if (p.includes('settings.local.json')) return JSON.stringify({ permissions: { allow: [] } }); + return JSON.stringify(settings); + }); + + const result = await hooksClaudeCountCheck.run(mockContext); + expect(result.status).toBe('PASS'); + expect(result.message).toContain('5 hook files found, 3 registered'); + }); + + it('should WARN when neither settings file registers any hook', async () => { + fs.existsSync.mockReturnValue(true); + fs.readdirSync.mockReturnValue([fileEntry('hook-a.cjs'), fileEntry('hook-b.cjs')]); + fs.readFileSync.mockReturnValue(JSON.stringify({ hooks: {} })); + + const result = await hooksClaudeCountCheck.run(mockContext); + expect(result.status).toBe('WARN'); + expect(result.message).toContain('settings.json or settings.local.json'); + }); + + it('should tolerate unparseable settings files', async () => { + fs.existsSync.mockReturnValue(true); + fs.readdirSync.mockReturnValue([fileEntry('hook-a.cjs'), fileEntry('hook-b.cjs')]); + fs.readFileSync.mockReturnValue('{ not valid json'); + + const result = await hooksClaudeCountCheck.run(mockContext); + expect(result.status).toBe('WARN'); + expect(result.message).toContain('not registered'); + }); }); // === INS-4.8: Registry and task validation === From f64baa8b2be137e6662be0c159dc3d67da828343 Mon Sep 17 00:00:00 2001 From: Flavio Tavares Date: Mon, 10 Aug 2026 21:28:46 +0100 Subject: [PATCH 2/2] fix(doctor): match hook filenames as complete path tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Substring matching treated sync.cjs as registered when a command only referenced sync-wrapper.cjs, so a missing registration could produce a PASS with an inflated registered count. Commands are now tokenized: quotes and trailing shell punctuation are stripped, Windows separators normalized, and only complete .cjs basenames count as a reference. Addresses CodeRabbit review on #821. Tests: 3 new cases (62 total) — similarly named files, suffix-only references, and quoted/backslashed paths. Verified the first two fail against the previous substring matching. Co-Authored-By: Claude Opus 5 (1M context) --- .../core/doctor/checks/hooks-claude-count.js | 32 +++++++++-- .aiox-core/install-manifest.yaml | 6 +-- .../tests/unit/doctor/doctor-checks.test.js | 53 +++++++++++++++++++ 3 files changed, 83 insertions(+), 8 deletions(-) diff --git a/.aiox-core/core/doctor/checks/hooks-claude-count.js b/.aiox-core/core/doctor/checks/hooks-claude-count.js index 0c7737f008..7b12eee6a4 100644 --- a/.aiox-core/core/doctor/checks/hooks-claude-count.js +++ b/.aiox-core/core/doctor/checks/hooks-claude-count.js @@ -54,6 +54,30 @@ function collectHookCommands(settingsPath) { return commands; } +/** + * Extracts referenced hook filenames from command strings. + * + * Matches complete .cjs filenames at path/shell-token boundaries rather than + * by substring: a bare `includes()` would treat `sync.cjs` as referenced by a + * command that only mentions `sync-wrapper.cjs`, turning a missing + * registration into a PASS with an inflated count. + */ +function referencedHookNames(commands) { + const names = new Set(); + + for (const command of commands) { + for (const rawToken of command.split(/\s+/)) { + // Strip surrounding quotes and trailing shell punctuation + const token = rawToken.replace(/^['"`(]+/, '').replace(/['"`;,)]+$/, ''); + if (!token.endsWith('.cjs')) continue; + // Normalize Windows separators before taking the basename + names.add(path.posix.basename(token.replace(/\\/g, '/'))); + } + } + + return names; +} + async function run(context) { const hooksDir = path.join(context.projectRoot, '.claude', 'hooks'); @@ -96,14 +120,12 @@ async function run(context) { // Wrapper hooks are registered directly; engine hooks they spawn as child // processes are not, so any reference is enough to count as wired up. const claudeDir = path.join(context.projectRoot, '.claude'); - const hooksStr = [ + const referenced = referencedHookNames([ ...collectHookCommands(path.join(claudeDir, 'settings.json')), ...collectHookCommands(path.join(claudeDir, 'settings.local.json')), - ].join('\n'); + ]); - const referencedCount = hookFiles.filter( - (f) => hooksStr.includes(f.name) || hooksStr.includes(f.name.replace('.cjs', '')), - ).length; + const referencedCount = hookFiles.filter((f) => referenced.has(f.name)).length; const registered = referencedCount > 0; if (hookCount >= 2 && registered) { diff --git a/.aiox-core/install-manifest.yaml b/.aiox-core/install-manifest.yaml index 5cf09028bb..d0b3fc4523 100644 --- a/.aiox-core/install-manifest.yaml +++ b/.aiox-core/install-manifest.yaml @@ -8,7 +8,7 @@ # - File types for categorization # version: 5.3.0 -generated_at: "2026-08-10T20:08:23.689Z" +generated_at: "2026-08-10T20:28:46.967Z" generator: scripts/generate-install-manifest.js file_count: 1165 files: @@ -337,9 +337,9 @@ files: type: core size: 1106 - path: core/doctor/checks/hooks-claude-count.js - hash: sha256:94fd5136635b0eb766c0dfc7ccbcff19bc2d63dc5dea9fedd2e16f21ce609875 + hash: sha256:fd5d7750e527b22d1e5caf212cecb025a5c62b6d28b0eecbe39e8e875ae8bd16 type: core - size: 3824 + size: 4629 - path: core/doctor/checks/ide-sync.js hash: sha256:4ddd037b4ad18c4201ca1428a1044efd313e9d2721cd399aebd3c5043fd4e2d1 type: core diff --git a/packages/installer/tests/unit/doctor/doctor-checks.test.js b/packages/installer/tests/unit/doctor/doctor-checks.test.js index 123a5b8f9b..f320ca33f4 100644 --- a/packages/installer/tests/unit/doctor/doctor-checks.test.js +++ b/packages/installer/tests/unit/doctor/doctor-checks.test.js @@ -777,6 +777,59 @@ describe('hooks-claude-count check', () => { expect(result.message).toContain('settings.json or settings.local.json'); }); + it('should not count a hook as registered via a longer similarly named file', async () => { + fs.existsSync.mockReturnValue(true); + fs.readdirSync.mockReturnValue([fileEntry('sync.cjs'), fileEntry('sync-wrapper.cjs')]); + const settings = { + hooks: { + UserPromptSubmit: [ + { hooks: [{ type: 'command', command: 'node .claude/hooks/sync-wrapper.cjs' }] }, + ], + }, + }; + fs.readFileSync.mockReturnValue(JSON.stringify(settings)); + + const result = await hooksClaudeCountCheck.run(mockContext); + // sync.cjs is NOT registered — only sync-wrapper.cjs is + expect(result.status).toBe('PASS'); + expect(result.message).toContain('2 hook files found, 1 registered'); + }); + + it('should not count a hook referenced only as a filename suffix', async () => { + fs.existsSync.mockReturnValue(true); + fs.readdirSync.mockReturnValue([fileEntry('sync.cjs'), fileEntry('hook-b.cjs')]); + const settings = { + hooks: { + UserPromptSubmit: [ + { hooks: [{ type: 'command', command: 'node .claude/hooks/legacy-sync.cjs' }] }, + ], + }, + }; + fs.readFileSync.mockReturnValue(JSON.stringify(settings)); + + const result = await hooksClaudeCountCheck.run(mockContext); + expect(result.status).toBe('WARN'); + expect(result.message).toContain('not registered'); + }); + + it('should match hook filenames despite quoting and trailing punctuation', async () => { + fs.existsSync.mockReturnValue(true); + fs.readdirSync.mockReturnValue([fileEntry('hook-a.cjs'), fileEntry('hook-b.cjs')]); + const settings = { + hooks: { + UserPromptSubmit: [ + { hooks: [{ type: 'command', command: 'node "$DIR/.claude/hooks/hook-a.cjs";' }] }, + { hooks: [{ type: 'command', command: 'node .claude\\hooks\\hook-b.cjs' }] }, + ], + }, + }; + fs.readFileSync.mockReturnValue(JSON.stringify(settings)); + + const result = await hooksClaudeCountCheck.run(mockContext); + expect(result.status).toBe('PASS'); + expect(result.message).toContain('2 registered'); + }); + it('should tolerate unparseable settings files', async () => { fs.existsSync.mockReturnValue(true); fs.readdirSync.mockReturnValue([fileEntry('hook-a.cjs'), fileEntry('hook-b.cjs')]);