diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..f6f8884 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,27 @@ +name: test + +on: + push: + branches: [main] + pull_request: + +jobs: + hooks: + name: hooks (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + # Windows is included on purpose: hook commands run through Git Bash + # there, which is where the PowerShell-in-a-bash-shell bug surfaced. + os: [ubuntu-latest, windows-latest] + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + # 22+: the test script relies on node's own --test glob expansion, + # since cmd.exe (npm's default script shell on Windows) can't glob. + node-version: 22 + - name: Run tests + run: npm test + shell: bash diff --git a/bin/fableit.js b/bin/fableit.js index 29a1d5f..fa6e1c1 100755 --- a/bin/fableit.js +++ b/bin/fableit.js @@ -23,8 +23,9 @@ const OPENCODE_CONFIG = path.join(getOpencodeDir(), 'opencode.json'); // An entry/value is ours iff it references the installed copy — never match by // the bare substring 'fableit', which could appear in a user's own paths. -const ownMarker = path.join(DEST, path.sep === '\\' ? 'hooks\\' : 'hooks/'); -const isOurs = v => JSON.stringify(v).includes(JSON.stringify(ownMarker).slice(1, -1)); +const norm = s => String(s).replace(/[\\/]+/g, '/'); +const ownMarker = norm(path.join(DEST, 'hooks')) + '/'; +const isOurs = v => norm(JSON.stringify(v)).includes(ownMarker); function fatal(msg) { console.error('fableit: ' + msg); @@ -73,9 +74,11 @@ function hookEntries() { const entry = { hooks: hooks.map(h => ({ type: h.type, - command: (process.platform === 'win32' && h.commandWindows - ? h.commandWindows.replace(/\$env:CLAUDE_PLUGIN_ROOT/g, DEST) - : h.command.replace(/\$\{CLAUDE_PLUGIN_ROOT\}/g, DEST)), + // hooks.json is the single source of truth and its `command` is bash; + // Claude Code runs every hook through bash (Git Bash on Windows), so + // substitute the plugin-root placeholder and normalise to '/'. node + // accepts forward slashes on Windows, keeping the command valid bash. + command: h.command.replace(/\$\{CLAUDE_PLUGIN_ROOT\}/g, norm(DEST)), timeout: h.timeout, })), }; @@ -86,6 +89,25 @@ function hookEntries() { return entries; } +// Reconcile our hooks within a settings object: drop any previous fableit +// entries (including a stale or broken one written by an older version) and +// add the current correct ones. Makes re-running install idempotent and +// self-healing, so a user never has to hand-remove a bad entry. +function reconcileClaudeHooks(settings, desired) { + settings.hooks = settings.hooks || {}; + for (const [event, entries] of Object.entries(desired)) { + const kept = (settings.hooks[event] || []).filter(e => !isOurs(e)); + settings.hooks[event] = [...kept, ...entries]; + } + // Also strip our entries from events we no longer register. + for (const event of Object.keys(settings.hooks)) { + if (desired[event]) continue; + settings.hooks[event] = settings.hooks[event].filter(e => !isOurs(e)); + if (!settings.hooks[event].length) delete settings.hooks[event]; + } + return settings; +} + function installClaude() { copyPackage(); @@ -97,11 +119,7 @@ function installClaude() { fs.copyFileSync(path.join(SRC, 'SKILL.md'), path.join(SKILLS_DIR, 'fableit', 'SKILL.md')); const settings = readConfigOrDie(SETTINGS, 'Claude settings'); - settings.hooks = settings.hooks || {}; - for (const [event, entries] of Object.entries(hookEntries())) { - const list = (settings.hooks[event] = settings.hooks[event] || []); - if (!list.some(isOurs)) list.push(...entries); - } + reconcileClaudeHooks(settings, hookEntries()); // Statusline badge — only if the user has none; never clobber an existing one. if (!settings.statusLine && process.platform !== 'win32') { settings.statusLine = { @@ -159,18 +177,23 @@ function uninstall() { console.log('fableit uninstalled (Claude Code hooks, skill, flag, OpenCode entry, installed copy).'); } -const cmd = process.argv[2] || 'claude'; -switch (cmd) { - case 'claude': - case 'install': - installClaude(); break; - case 'opencode': - installOpencode(); break; - case 'print': - console.log(getFableitInstructions(process.argv[3] || 'full')); break; - case 'uninstall': - uninstall(); break; - default: - console.log('usage: npx fableit [claude|opencode|print [lite|full|ultra]|uninstall]'); - process.exit(1); +if (require.main === module) { + const cmd = process.argv[2] || 'claude'; + switch (cmd) { + case 'claude': + case 'install': + installClaude(); break; + case 'opencode': + installOpencode(); break; + case 'print': + console.log(getFableitInstructions(process.argv[3] || 'full')); break; + case 'uninstall': + uninstall(); break; + default: + console.log('usage: npx fableit [claude|opencode|print [lite|full|ultra]|uninstall]'); + process.exit(1); + } } + +// Exported for tests; the CLI above only runs when the file is executed directly. +module.exports = { hookEntries, reconcileClaudeHooks, isOurs }; diff --git a/hooks/hooks.json b/hooks/hooks.json index 26e9c9c..2d0e514 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -7,7 +7,6 @@ { "type": "command", "command": "node \"${CLAUDE_PLUGIN_ROOT}/hooks/fableit-activate.js\"; exit 0", - "commandWindows": "if (Get-Command node -ErrorAction SilentlyContinue) { node \"$env:CLAUDE_PLUGIN_ROOT\\hooks\\fableit-activate.js\" }", "timeout": 5, "statusMessage": "Loading fableit mode..." } @@ -20,7 +19,6 @@ { "type": "command", "command": "node \"${CLAUDE_PLUGIN_ROOT}/hooks/fableit-subagent.js\"; exit 0", - "commandWindows": "if (Get-Command node -ErrorAction SilentlyContinue) { node \"$env:CLAUDE_PLUGIN_ROOT\\hooks\\fableit-subagent.js\" }", "timeout": 5, "statusMessage": "Loading fableit mode..." } @@ -33,7 +31,6 @@ { "type": "command", "command": "node \"${CLAUDE_PLUGIN_ROOT}/hooks/fableit-mode-tracker.js\"; exit 0", - "commandWindows": "if (Get-Command node -ErrorAction SilentlyContinue) { node \"$env:CLAUDE_PLUGIN_ROOT\\hooks\\fableit-mode-tracker.js\" }", "timeout": 5, "statusMessage": "Tracking fableit mode..." } diff --git a/package.json b/package.json index 44dc082..2eb80a3 100644 --- a/package.json +++ b/package.json @@ -42,7 +42,7 @@ "opencode.json" ], "scripts": { - "test": "node --test tests/*.test.js", + "test": "node --test \"tests/*.test.js\"", "publish:both": "npm publish && npm pkg set name=fableit && npm publish && npm pkg set name=@seedexr/fableit" }, "publishConfig": { diff --git a/tests/fixtures/hooks.broken.json b/tests/fixtures/hooks.broken.json new file mode 100644 index 0000000..fd9dfc8 --- /dev/null +++ b/tests/fixtures/hooks.broken.json @@ -0,0 +1,17 @@ +{ + "hooks": { + "UserPromptSubmit": [ + { + "hooks": [ + { + "type": "command", + "command": "node \"${CLAUDE_PLUGIN_ROOT}/hooks/fableit-mode-tracker.js\"; exit 0", + "commandWindows": "if (Get-Command node -ErrorAction SilentlyContinue) { node \"$env:CLAUDE_PLUGIN_ROOT\\hooks\\fableit-mode-tracker.js\" }", + "timeout": 5, + "statusMessage": "Tracking fableit mode..." + } + ] + } + ] + } +} diff --git a/tests/hooks.test.js b/tests/hooks.test.js new file mode 100644 index 0000000..860086a --- /dev/null +++ b/tests/hooks.test.js @@ -0,0 +1,141 @@ +'use strict'; + +// fableit — hooks.json integrity + hook-script behaviour. +// +// Why this file exists: Claude Code runs every hook command through bash on +// ALL platforms (including Windows/Git Bash). A hook whose command isn't valid +// bash, or a UserPromptSubmit hook that exits non-zero, silently BLOCKS every +// prompt. That is exactly the regression this suite guards against. See the +// self-check at the bottom, which asserts the guard actually has teeth against +// a known-bad fixture. + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const { execFileSync, execSync } = require('node:child_process'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); + +const ROOT = path.resolve(__dirname, '..'); +const HOOKS_DIR = path.join(ROOT, 'hooks'); +const HOOKS_JSON = path.join(HOOKS_DIR, 'hooks.json'); + +// Fields that Claude Code will hand to a shell. `commandWindows` is a legacy +// field some hosts pick up on Windows; if present it is ALSO run through bash, +// so it must be valid bash too. Validate every one of them. +const SHELL_FIELDS = ['command', 'commandWindows']; + +function bashAvailable() { + try { execFileSync('bash', ['-c', 'true'], { stdio: 'ignore' }); return true; } + catch { return false; } +} + +// Returns a list of human-readable problems. Empty list = the file is safe. +function validateHooksFile(file) { + const problems = []; + let parsed; + try { + parsed = JSON.parse(fs.readFileSync(file, 'utf8').replace(/^\uFEFF/, '')); + } catch (e) { + return [`invalid JSON: ${e.message}`]; + } + const events = parsed && parsed.hooks; + if (!events || typeof events !== 'object') return ['missing top-level "hooks" object']; + + for (const [event, groups] of Object.entries(events)) { + if (!Array.isArray(groups)) { problems.push(`${event}: expected an array`); continue; } + groups.forEach((group, gi) => { + const handlers = group && group.hooks; + if (!Array.isArray(handlers)) { problems.push(`${event}[${gi}]: missing "hooks" array`); return; } + handlers.forEach((h, hi) => { + const where = `${event}[${gi}].hooks[${hi}]`; + if (h.type !== 'command') { problems.push(`${where}: type must be "command"`); return; } + if (typeof h.command !== 'string' || !h.command.trim()) { + problems.push(`${where}: "command" must be a non-empty string`); + } + // Every shell-bearing field must parse as bash. This is the check that + // catches PowerShell (or any non-bash) syntax like `if (...) { }`. + for (const field of SHELL_FIELDS) { + if (typeof h[field] !== 'string') continue; + try { + execFileSync('bash', ['-n', '-c', h[field]], { stdio: 'pipe' }); + } catch (e) { + const msg = (e.stderr ? e.stderr.toString() : e.message).trim().split('\n').pop(); + problems.push(`${where}.${field}: not valid bash -> ${msg}`); + } + } + // The referenced script must exist in hooks/. + const m = /([A-Za-z0-9_.-]+\.js)/.exec(h.command || ''); + if (m && !fs.existsSync(path.join(HOOKS_DIR, m[1]))) { + problems.push(`${where}: references missing script hooks/${m[1]}`); + } + }); + }); + } + return problems; +} + +// Run a hook script with a mock stdin payload in an isolated config dir, so we +// never touch the developer's real ~/.claude/.fableit-active flag. +function runHook(script, stdin) { + const sandbox = fs.mkdtempSync(path.join(os.tmpdir(), 'fableit-test-')); + try { + const out = execSync(`node "${path.join(HOOKS_DIR, script)}"`, { + input: stdin, + env: { ...process.env, CLAUDE_CONFIG_DIR: sandbox, FABLEIT_DEFAULT_MODE: 'full' }, + stdio: ['pipe', 'pipe', 'pipe'], + }); + return { code: 0, stdout: out.toString() }; + } catch (e) { + return { code: e.status == null ? 1 : e.status, stdout: (e.stdout || '').toString() }; + } finally { + fs.rmSync(sandbox, { recursive: true, force: true }); + } +} + +test('bash is available to run the shell-syntax checks', () => { + assert.ok(bashAvailable(), + 'bash not found on PATH. Claude Code runs hooks through bash, and so does ' + + 'this test. On Windows, run via Git Bash (bundled with Git for Windows).'); +}); + +test('hooks.json is present and valid (JSON, shape, bash-parseable, scripts exist)', () => { + assert.ok(fs.existsSync(HOOKS_JSON), 'hooks/hooks.json is missing'); + const problems = validateHooksFile(HOOKS_JSON); + assert.deepEqual(problems, [], 'hooks.json problems:\n' + problems.join('\n')); +}); + +test('UserPromptSubmit hook never blocks a prompt (always exits 0)', () => { + const cases = [ + '{"prompt":"/fableit ultra"}', // a real mode switch + '{"prompt":"how do I center a div"}', // an ordinary prompt + '{"prompt":""}', // empty prompt field + 'this is not json at all', // garbage stdin + '', // empty stdin + ]; + for (const stdin of cases) { + const { code } = runHook('fableit-mode-tracker.js', stdin); + assert.equal(code, 0, `mode-tracker exited ${code} on stdin: ${JSON.stringify(stdin)}`); + } +}); + +test('mode switch emits guidance to stdout', () => { + const { code, stdout } = runHook('fableit-mode-tracker.js', '{"prompt":"/fableit ultra"}'); + assert.equal(code, 0); + assert.ok(stdout.trim().length > 0, 'expected a ruleset/directive on stdout for a mode switch'); +}); + +test('SessionStart and SubagentStart hooks exit 0', () => { + assert.equal(runHook('fableit-activate.js', '{}').code, 0, 'activate must exit 0'); + assert.equal(runHook('fableit-subagent.js', '{}').code, 0, 'subagent must exit 0'); +}); + +// Self-check: the guard must actually reject the pre-fix file. Without this, +// a validator that silently passes everything would look "green" forever. +test('validator rejects the known-bad fixture (PowerShell command run via bash)', () => { + const bad = path.join(__dirname, 'fixtures', 'hooks.broken.json'); + const problems = validateHooksFile(bad); + assert.ok(problems.length > 0, 'expected the broken fixture to be rejected'); + assert.ok(problems.some(p => /commandWindows/.test(p) && /not valid bash/.test(p)), + 'expected a bash-syntax rejection on the commandWindows field, got:\n' + problems.join('\n')); +}); diff --git a/tests/installer.test.js b/tests/installer.test.js new file mode 100644 index 0000000..4f5e94a --- /dev/null +++ b/tests/installer.test.js @@ -0,0 +1,95 @@ +'use strict'; + +// fableit — installer (bin/fableit.js) hook reconciliation. +// +// Guards the "self-healing" behaviour: re-running the installer must replace any +// previous fableit hook entry in settings.json (including a stale PowerShell one +// from an older version) with the current, bash-valid entry — without touching +// the user's own unrelated hooks, and without leaving duplicates. + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const { execFileSync } = require('node:child_process'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); + +// getClaudeDir() (used inside the installer to compute DEST/SETTINGS) honours +// CLAUDE_CONFIG_DIR. Set it to a temp dir BEFORE requiring the module so every +// derived path is sandboxed and deterministic. +const SANDBOX = fs.mkdtempSync(path.join(os.tmpdir(), 'fableit-install-')); +process.env.CLAUDE_CONFIG_DIR = SANDBOX; + +const installer = require('../bin/fableit.js'); +const DEST_HOOKS = path.join(SANDBOX, 'fableit', 'hooks'); // where our scripts live + +function bashOK(cmd) { + try { execFileSync('bash', ['-n', '-c', cmd], { stdio: 'pipe' }); return true; } + catch { return false; } +} + +// A settings.json as an older install would have left it on Windows: a stale +// PowerShell UserPromptSubmit entry pointing at our install dir (this is the +// exact shape that blocked every prompt), plus a user's own unrelated hook. +function staleSettings() { + const winPath = path.join(DEST_HOOKS, 'fableit-mode-tracker.js'); + return { + hooks: { + UserPromptSubmit: [ + { + hooks: [{ + type: 'command', + command: `if (Get-Command node -ErrorAction SilentlyContinue) { node "${winPath}" }`, + timeout: 5, + }], + }, + { + // the user's own hook — must survive untouched + hooks: [{ type: 'command', command: 'echo "my own hook"' }], + }, + ], + }, + }; +} + +test('reconcile replaces a stale PowerShell entry with a valid bash entry', () => { + const settings = staleSettings(); + installer.reconcileClaudeHooks(settings, installer.hookEntries()); + + const ups = settings.hooks.UserPromptSubmit; + const ours = ups.filter(installer.isOurs); + const theirs = ups.filter(e => !installer.isOurs(e)); + + // exactly one of ours (no duplicate), user's hook preserved + assert.equal(ours.length, 1, 'expected exactly one fableit UserPromptSubmit entry'); + assert.equal(theirs.length, 1, "user's own hook must be preserved"); + assert.equal(theirs[0].hooks[0].command, 'echo "my own hook"'); + + // the surviving fableit command is bash-valid and carries no PowerShell + const cmd = ours[0].hooks[0].command; + assert.ok(!/Get-Command|SilentlyContinue/.test(cmd), 'PowerShell must be gone'); + assert.ok(bashOK(cmd), `fableit command must be valid bash, got: ${cmd}`); + assert.match(cmd, /; exit 0$/, 'command must end with "; exit 0" so it never blocks'); +}); + +test('reconcile is idempotent (running twice changes nothing)', () => { + const settings = { hooks: {} }; + installer.reconcileClaudeHooks(settings, installer.hookEntries()); + const once = JSON.stringify(settings); + installer.reconcileClaudeHooks(settings, installer.hookEntries()); + assert.equal(JSON.stringify(settings), once, 'second run should be a no-op'); +}); + +test('every event our installer emits is valid bash', () => { + const settings = { hooks: {} }; + installer.reconcileClaudeHooks(settings, installer.hookEntries()); + for (const [event, groups] of Object.entries(settings.hooks)) { + for (const g of groups) { + for (const h of g.hooks) { + assert.ok(bashOK(h.command), `${event}: not valid bash -> ${h.command}`); + } + } + } +}); + +test.after(() => fs.rmSync(SANDBOX, { recursive: true, force: true }));