From 3fadc37802619ec196da8e144029289ad92bcbd2 Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Wed, 29 Apr 2026 21:18:29 -0400 Subject: [PATCH] fix: route continuous learning observe hooks through node --- docs/ARCHITECTURE-IMPROVEMENTS.md | 2 +- hooks/README.md | 2 +- hooks/hooks.json | 4 +- scripts/hooks/observe-runner.js | 196 ++++++++++++++++++ scripts/hooks/run-with-flags.js | 11 +- ...continuous-learning-observe-runner.test.js | 194 +++++++++++++++++ 6 files changed, 404 insertions(+), 5 deletions(-) create mode 100644 scripts/hooks/observe-runner.js create mode 100644 tests/hooks/continuous-learning-observe-runner.test.js diff --git a/docs/ARCHITECTURE-IMPROVEMENTS.md b/docs/ARCHITECTURE-IMPROVEMENTS.md index f8a5e605..5a2803e5 100644 --- a/docs/ARCHITECTURE-IMPROVEMENTS.md +++ b/docs/ARCHITECTURE-IMPROVEMENTS.md @@ -110,7 +110,7 @@ This document captures architect-level improvements for the Everything Claude Co ### 5.1 Hook Runtime Consistency -**Issue:** Most hooks invoke Node scripts via `run-with-flags.js`; one path uses `run-with-flags-shell.sh` + `observe.sh`. The mixed runtime is documented but could be simplified over time. +**Issue:** Hooks should keep a consistent Node-mode dispatch surface. Continuous-learning observation now dispatches through `run-with-flags.js` and `observe-runner.js`, which delegates to the existing `observe.sh` implementation without exposing a shell-mode hook entry. **Recommendation:** diff --git a/hooks/README.md b/hooks/README.md index d712d94f..0d038326 100644 --- a/hooks/README.md +++ b/hooks/README.md @@ -228,7 +228,7 @@ Async hooks run in the background. They cannot block tool execution. ## Cross-Platform Notes -Hook logic is implemented in Node.js scripts for cross-platform behavior on Windows, macOS, and Linux. A small number of shell wrappers are retained for continuous-learning observer hooks; those wrappers are profile-gated and have Windows-safe fallback behavior. +Hook logic is implemented in Node.js scripts for cross-platform behavior on Windows, macOS, and Linux. The continuous-learning observer is exposed as a Node-mode hook and delegates to its existing `observe.sh` implementation through a profile-gated runner with Windows-safe fallback behavior. ## Related diff --git a/hooks/hooks.json b/hooks/hooks.json index ceb3e6c8..5c5aad84 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -40,7 +40,7 @@ "hooks": [ { "type": "command", - "command": "node -e \"const p=require('path');const r=(()=>{var e=process.env.CLAUDE_PLUGIN_ROOT;if(e&&e.trim())return e.trim();var p=require('path'),f=require('fs'),h=require('os').homedir(),d=p.join(h,'.claude'),q=p.join('scripts','lib','utils.js');if(f.existsSync(p.join(d,q)))return d;for(var s of [[\\\"ecc\\\"],[\\\"ecc@ecc\\\"],[\\\"marketplace\\\",\\\"ecc\\\"],[\\\"everything-claude-code\\\"],[\\\"everything-claude-code@everything-claude-code\\\"],[\\\"marketplace\\\",\\\"everything-claude-code\\\"]]){var l=p.join(d,'plugins',...s);if(f.existsSync(p.join(l,q)))return l}try{for(var g of [\\\"ecc\\\",\\\"everything-claude-code\\\"]){var b=p.join(d,'plugins','cache',g);for(var o of f.readdirSync(b,{withFileTypes:true})){if(!o.isDirectory())continue;for(var v of f.readdirSync(p.join(b,o.name),{withFileTypes:true})){if(!v.isDirectory())continue;var c=p.join(b,o.name,v.name);if(f.existsSync(p.join(c,q)))return c}}}}catch(x){}return d})();const s=p.join(r,'scripts/hooks/plugin-hook-bootstrap.js');process.env.CLAUDE_PLUGIN_ROOT=r;process.argv.splice(1,0,s);require(s)\" shell scripts/hooks/run-with-flags-shell.sh pre:observe skills/continuous-learning-v2/hooks/observe.sh standard,strict", + "command": "node -e \"const p=require('path');const r=(()=>{var e=process.env.CLAUDE_PLUGIN_ROOT;if(e&&e.trim())return e.trim();var p=require('path'),f=require('fs'),h=require('os').homedir(),d=p.join(h,'.claude'),q=p.join('scripts','lib','utils.js');if(f.existsSync(p.join(d,q)))return d;for(var s of [[\\\"ecc\\\"],[\\\"ecc@ecc\\\"],[\\\"marketplace\\\",\\\"ecc\\\"],[\\\"everything-claude-code\\\"],[\\\"everything-claude-code@everything-claude-code\\\"],[\\\"marketplace\\\",\\\"everything-claude-code\\\"]]){var l=p.join(d,'plugins',...s);if(f.existsSync(p.join(l,q)))return l}try{for(var g of [\\\"ecc\\\",\\\"everything-claude-code\\\"]){var b=p.join(d,'plugins','cache',g);for(var o of f.readdirSync(b,{withFileTypes:true})){if(!o.isDirectory())continue;for(var v of f.readdirSync(p.join(b,o.name),{withFileTypes:true})){if(!v.isDirectory())continue;var c=p.join(b,o.name,v.name);if(f.existsSync(p.join(c,q)))return c}}}}catch(x){}return d})();const s=p.join(r,'scripts/hooks/plugin-hook-bootstrap.js');process.env.CLAUDE_PLUGIN_ROOT=r;process.argv.splice(1,0,s);require(s)\" node scripts/hooks/run-with-flags.js pre:observe scripts/hooks/observe-runner.js standard,strict", "async": true, "timeout": 10 } @@ -212,7 +212,7 @@ "hooks": [ { "type": "command", - "command": "node -e \"const p=require('path');const r=(()=>{var e=process.env.CLAUDE_PLUGIN_ROOT;if(e&&e.trim())return e.trim();var p=require('path'),f=require('fs'),h=require('os').homedir(),d=p.join(h,'.claude'),q=p.join('scripts','lib','utils.js');if(f.existsSync(p.join(d,q)))return d;for(var s of [[\\\"ecc\\\"],[\\\"ecc@ecc\\\"],[\\\"marketplace\\\",\\\"ecc\\\"],[\\\"everything-claude-code\\\"],[\\\"everything-claude-code@everything-claude-code\\\"],[\\\"marketplace\\\",\\\"everything-claude-code\\\"]]){var l=p.join(d,'plugins',...s);if(f.existsSync(p.join(l,q)))return l}try{for(var g of [\\\"ecc\\\",\\\"everything-claude-code\\\"]){var b=p.join(d,'plugins','cache',g);for(var o of f.readdirSync(b,{withFileTypes:true})){if(!o.isDirectory())continue;for(var v of f.readdirSync(p.join(b,o.name),{withFileTypes:true})){if(!v.isDirectory())continue;var c=p.join(b,o.name,v.name);if(f.existsSync(p.join(c,q)))return c}}}}catch(x){}return d})();const s=p.join(r,'scripts/hooks/plugin-hook-bootstrap.js');process.env.CLAUDE_PLUGIN_ROOT=r;process.argv.splice(1,0,s);require(s)\" shell scripts/hooks/run-with-flags-shell.sh post:observe skills/continuous-learning-v2/hooks/observe.sh standard,strict", + "command": "node -e \"const p=require('path');const r=(()=>{var e=process.env.CLAUDE_PLUGIN_ROOT;if(e&&e.trim())return e.trim();var p=require('path'),f=require('fs'),h=require('os').homedir(),d=p.join(h,'.claude'),q=p.join('scripts','lib','utils.js');if(f.existsSync(p.join(d,q)))return d;for(var s of [[\\\"ecc\\\"],[\\\"ecc@ecc\\\"],[\\\"marketplace\\\",\\\"ecc\\\"],[\\\"everything-claude-code\\\"],[\\\"everything-claude-code@everything-claude-code\\\"],[\\\"marketplace\\\",\\\"everything-claude-code\\\"]]){var l=p.join(d,'plugins',...s);if(f.existsSync(p.join(l,q)))return l}try{for(var g of [\\\"ecc\\\",\\\"everything-claude-code\\\"]){var b=p.join(d,'plugins','cache',g);for(var o of f.readdirSync(b,{withFileTypes:true})){if(!o.isDirectory())continue;for(var v of f.readdirSync(p.join(b,o.name),{withFileTypes:true})){if(!v.isDirectory())continue;var c=p.join(b,o.name,v.name);if(f.existsSync(p.join(c,q)))return c}}}}catch(x){}return d})();const s=p.join(r,'scripts/hooks/plugin-hook-bootstrap.js');process.env.CLAUDE_PLUGIN_ROOT=r;process.argv.splice(1,0,s);require(s)\" node scripts/hooks/run-with-flags.js post:observe scripts/hooks/observe-runner.js standard,strict", "async": true, "timeout": 10 } diff --git a/scripts/hooks/observe-runner.js b/scripts/hooks/observe-runner.js new file mode 100644 index 00000000..28e7f4fb --- /dev/null +++ b/scripts/hooks/observe-runner.js @@ -0,0 +1,196 @@ +#!/usr/bin/env node +'use strict'; + +const fs = require('fs'); +const path = require('path'); +const { spawnSync } = require('child_process'); + +const OBSERVE_RELATIVE_PATH = path.join('skills', 'continuous-learning-v2', 'hooks', 'observe.sh'); +const DEFAULT_TIMEOUT_MS = 9000; + +function getPluginRoot(options = {}) { + if (options.pluginRoot && String(options.pluginRoot).trim()) { + return String(options.pluginRoot).trim(); + } + if (process.env.CLAUDE_PLUGIN_ROOT && process.env.CLAUDE_PLUGIN_ROOT.trim()) { + return process.env.CLAUDE_PLUGIN_ROOT.trim(); + } + if (process.env.ECC_PLUGIN_ROOT && process.env.ECC_PLUGIN_ROOT.trim()) { + return process.env.ECC_PLUGIN_ROOT.trim(); + } + return path.resolve(__dirname, '..', '..'); +} + +function resolveTarget(rootDir, relPath) { + const resolvedRoot = path.resolve(rootDir); + const resolvedTarget = path.resolve(rootDir, relPath); + if ( + resolvedTarget !== resolvedRoot && + !resolvedTarget.startsWith(resolvedRoot + path.sep) + ) { + throw new Error(`Path traversal rejected: ${relPath}`); + } + return resolvedTarget; +} + +function toShellPath(filePath) { + const normalized = String(filePath || ''); + if (process.platform !== 'win32') { + return normalized; + } + + return normalized + .replace(/^([A-Za-z]):[\\/]/, (_, driveLetter) => `/${driveLetter.toLowerCase()}/`) + .replace(/\\/g, '/'); +} + +function findShellBinary() { + const candidates = []; + if (process.env.BASH && process.env.BASH.trim()) { + candidates.push(process.env.BASH.trim()); + } + + if (process.platform === 'win32') { + candidates.push('bash.exe', 'bash', 'sh'); + } else { + candidates.push('bash', 'sh'); + } + + for (const candidate of candidates) { + const probe = spawnSync(candidate, ['-c', ':'], { + stdio: 'ignore', + windowsHide: true + }); + if (!probe.error) { + return candidate; + } + } + + return null; +} + +function getPhaseFromHookId(hookId) { + const prefix = String(hookId || process.env.ECC_HOOK_ID || '').split(':')[0]; + return prefix === 'pre' || prefix === 'post' ? prefix : null; +} + +function getTimeoutMs() { + const parsed = Number.parseInt(process.env.ECC_OBSERVE_RUNNER_TIMEOUT_MS || '', 10); + return Number.isFinite(parsed) && parsed > 0 ? parsed : DEFAULT_TIMEOUT_MS; +} + +function combineStderr(stderr, message) { + const prefix = typeof stderr === 'string' && stderr.length > 0 + ? stderr.endsWith('\n') ? stderr : `${stderr}\n` + : ''; + return `${prefix}${message}\n`; +} + +function run(raw, options = {}) { + const input = typeof raw === 'string' ? raw : String(raw ?? ''); + const phase = getPhaseFromHookId(options.hookId); + if (!phase) { + return { + stderr: '[Hook] observe runner received an unsupported hook id; skipping observation', + exitCode: 0 + }; + } + + const pluginRoot = getPluginRoot(options); + let observePath; + try { + observePath = resolveTarget(pluginRoot, OBSERVE_RELATIVE_PATH); + } catch (error) { + return { + stderr: `[Hook] observe runner path resolution failed: ${error.message}`, + exitCode: 0 + }; + } + + if (!fs.existsSync(observePath)) { + return { + stderr: `[Hook] observe script not found: ${observePath}`, + exitCode: 0 + }; + } + + const shell = findShellBinary(); + if (!shell) { + return { + stderr: '[Hook] shell runtime unavailable; skipping continuous-learning observation', + exitCode: 0 + }; + } + + const result = spawnSync(shell, [toShellPath(observePath), phase], { + input, + encoding: 'utf8', + env: { + ...process.env, + CLAUDE_PLUGIN_ROOT: pluginRoot, + ECC_PLUGIN_ROOT: pluginRoot + }, + cwd: process.cwd(), + timeout: getTimeoutMs(), + windowsHide: true + }); + + const output = { + exitCode: Number.isInteger(result.status) ? result.status : 0 + }; + + if (typeof result.stdout === 'string' && result.stdout.length > 0) { + output.stdout = result.stdout; + } + if (typeof result.stderr === 'string' && result.stderr.length > 0) { + output.stderr = result.stderr; + } + + if (result.error || result.signal || result.status === null) { + const reason = result.error + ? result.error.message + : result.signal + ? `terminated by signal ${result.signal}` + : 'missing exit status'; + output.stderr = combineStderr(output.stderr, `[Hook] observe runner failed: ${reason}`); + output.exitCode = 0; + } + + return output; +} + +function emitHookResult(raw, output) { + if (output && typeof output === 'object') { + if (output.stderr) { + process.stderr.write(String(output.stderr).endsWith('\n') ? String(output.stderr) : `${output.stderr}\n`); + } + if (Object.prototype.hasOwnProperty.call(output, 'stdout')) { + process.stdout.write(String(output.stdout ?? '')); + } else if (!Number.isInteger(output.exitCode) || output.exitCode === 0) { + process.stdout.write(raw); + } + return Number.isInteger(output.exitCode) ? output.exitCode : 0; + } + + process.stdout.write(raw); + return 0; +} + +if (require.main === module) { + let raw = ''; + try { + raw = fs.readFileSync(0, 'utf8'); + } catch (_error) { + raw = ''; + } + const output = run(raw, { hookId: process.argv[2] || process.env.ECC_HOOK_ID }); + process.exit(emitHookResult(raw, output)); +} + +module.exports = { + OBSERVE_RELATIVE_PATH, + findShellBinary, + getPhaseFromHookId, + run, + toShellPath +}; diff --git a/scripts/hooks/run-with-flags.js b/scripts/hooks/run-with-flags.js index 1391a454..84aed34a 100755 --- a/scripts/hooks/run-with-flags.js +++ b/scripts/hooks/run-with-flags.js @@ -137,7 +137,13 @@ async function main() { if (hookModule && typeof hookModule.run === 'function') { try { - const output = hookModule.run(raw, { truncated, maxStdin: MAX_STDIN }); + const output = hookModule.run(raw, { + hookId, + pluginRoot, + scriptPath, + truncated, + maxStdin: MAX_STDIN + }); process.exit(emitHookResult(raw, output)); } catch (runErr) { process.stderr.write(`[Hook] run() error for ${hookId}: ${runErr.message}\n`); @@ -152,6 +158,9 @@ async function main() { encoding: 'utf8', env: { ...process.env, + CLAUDE_PLUGIN_ROOT: pluginRoot, + ECC_PLUGIN_ROOT: pluginRoot, + ECC_HOOK_ID: hookId, ECC_HOOK_INPUT_TRUNCATED: truncated ? '1' : '0', ECC_HOOK_INPUT_MAX_BYTES: String(MAX_STDIN) }, diff --git a/tests/hooks/continuous-learning-observe-runner.test.js b/tests/hooks/continuous-learning-observe-runner.test.js new file mode 100644 index 00000000..5abb1907 --- /dev/null +++ b/tests/hooks/continuous-learning-observe-runner.test.js @@ -0,0 +1,194 @@ +/** + * Tests for continuous-learning-v2 observe hook dispatch. + * + * Run with: node tests/hooks/continuous-learning-observe-runner.test.js + */ + +'use strict'; + +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { spawnSync } = require('child_process'); + +const repoRoot = path.resolve(__dirname, '..', '..'); +const hooksJsonPath = path.join(repoRoot, 'hooks', 'hooks.json'); +const runWithFlagsPath = path.join(repoRoot, 'scripts', 'hooks', 'run-with-flags.js'); +const observeRunner = require(path.join(repoRoot, 'scripts', 'hooks', 'observe-runner.js')); + +function test(name, fn) { + try { + fn(); + console.log(` \u2713 ${name}`); + return true; + } catch (err) { + console.log(` \u2717 ${name}`); + console.log(` Error: ${err.message}`); + return false; + } +} + +function loadHook(id) { + const hookGroups = JSON.parse(fs.readFileSync(hooksJsonPath, 'utf8')).hooks; + const hooks = Object.values(hookGroups).flat(); + const hook = hooks.find(candidate => candidate.id === id); + assert.ok(hook, `Expected ${id} in hooks/hooks.json`); + assert.ok(Array.isArray(hook.hooks), `Expected ${id} to define hook commands`); + assert.strictEqual(hook.hooks.length, 1, `Expected ${id} to have one command`); + return hook.hooks[0].command; +} + +function withTempPluginRoot(fn) { + const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-observe-runner-')); + try { + fs.mkdirSync(path.join(tempRoot, 'scripts', 'hooks'), { recursive: true }); + fs.mkdirSync(path.join(tempRoot, 'scripts', 'lib'), { recursive: true }); + fs.copyFileSync( + path.join(repoRoot, 'scripts', 'lib', 'hook-flags.js'), + path.join(tempRoot, 'scripts', 'lib', 'hook-flags.js') + ); + return fn(tempRoot); + } finally { + fs.rmSync(tempRoot, { recursive: true, force: true }); + } +} + +function withEnv(vars, fn) { + const saved = {}; + for (const [key, value] of Object.entries(vars)) { + saved[key] = process.env[key]; + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + + try { + return fn(); + } finally { + for (const [key, value] of Object.entries(saved)) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + } +} + +function writeFakeObserveScript(tempRoot) { + const scriptPath = path.join(tempRoot, 'skills', 'continuous-learning-v2', 'hooks', 'observe.sh'); + fs.mkdirSync(path.dirname(scriptPath), { recursive: true }); + fs.writeFileSync( + scriptPath, + [ + '#!/usr/bin/env bash', + 'input="$(cat)"', + 'printf "phase=%s input=%s root=%s" "$1" "$input" "${CLAUDE_PLUGIN_ROOT:-}"', + '' + ].join('\n'), + 'utf8' + ); + fs.chmodSync(scriptPath, 0o755); +} + +function runWithFlags(tempRoot, hookId, relScriptPath, stdin) { + return spawnSync(process.execPath, [runWithFlagsPath, hookId, relScriptPath, 'standard,strict'], { + input: stdin, + encoding: 'utf8', + env: { + ...process.env, + CLAUDE_PLUGIN_ROOT: tempRoot, + ECC_HOOK_PROFILE: 'standard' + }, + stdio: ['pipe', 'pipe', 'pipe'], + timeout: 10000 + }); +} + +function runTests() { + console.log('\n=== Testing continuous-learning observe hook dispatch ===\n'); + + let passed = 0; + let failed = 0; + + if (test('observe hooks use node-mode runner instead of shell-mode dispatch', () => { + for (const hookId of ['pre:observe:continuous-learning', 'post:observe:continuous-learning']) { + const command = loadHook(hookId); + const phase = hookId.startsWith('pre:') ? 'pre:observe' : 'post:observe'; + + assert.ok(command.includes(`node scripts/hooks/run-with-flags.js ${phase} scripts/hooks/observe-runner.js standard,strict`)); + assert.ok(!command.includes('shell scripts/hooks/run-with-flags-shell.sh'), `${hookId} should not use shell-mode bootstrap`); + assert.ok(!command.includes('skills/continuous-learning-v2/hooks/observe.sh'), `${hookId} should not call observe.sh directly from hooks.json`); + } + })) passed++; else failed++; + + if (test('run-with-flags passes hookId to direct run exports', () => { + withTempPluginRoot(tempRoot => { + const scriptPath = path.join(tempRoot, 'scripts', 'hooks', 'capture-hook-id.js'); + fs.writeFileSync( + scriptPath, + [ + "'use strict';", + 'module.exports.run = function run(raw, options) {', + ' return { stdout: JSON.stringify({ raw, hookId: options.hookId, truncated: options.truncated }) };', + '};', + '' + ].join('\n'), + 'utf8' + ); + + const result = runWithFlags(tempRoot, 'post:observe', 'scripts/hooks/capture-hook-id.js', '{"ok":true}'); + assert.strictEqual(result.status, 0, result.stderr); + const payload = JSON.parse(result.stdout); + assert.deepStrictEqual(payload, { raw: '{"ok":true}', hookId: 'post:observe', truncated: false }); + }); + })) passed++; else failed++; + + if (test('observe-runner derives the observe phase from the hook id', () => { + assert.strictEqual(observeRunner.getPhaseFromHookId('pre:observe'), 'pre'); + assert.strictEqual(observeRunner.getPhaseFromHookId('post:observe'), 'post'); + assert.strictEqual(observeRunner.getPhaseFromHookId('pre:observe:continuous-learning'), 'pre'); + assert.strictEqual(observeRunner.getPhaseFromHookId('unknown'), null); + })) passed++; else failed++; + + if (test('observe-runner invokes observe.sh with phase, stdin, and plugin root', () => { + withTempPluginRoot(tempRoot => { + writeFakeObserveScript(tempRoot); + const env = fs.existsSync('/bin/sh') ? { BASH: '/bin/sh' } : {}; + withEnv(env, () => { + const output = observeRunner.run('payload', { + hookId: 'pre:observe', + pluginRoot: tempRoot + }); + + assert.strictEqual(output.exitCode, 0, output.stderr); + assert.strictEqual(output.stdout, `phase=pre input=payload root=${tempRoot}`); + }); + }); + })) passed++; else failed++; + + if (test('observe-runner fails open when no shell runtime is available', () => { + withTempPluginRoot(tempRoot => { + writeFakeObserveScript(tempRoot); + withEnv({ BASH: '', PATH: '' }, () => { + const output = observeRunner.run('payload', { + hookId: 'post:observe', + pluginRoot: tempRoot + }); + + assert.strictEqual(output.exitCode, 0); + assert.ok(!Object.prototype.hasOwnProperty.call(output, 'stdout'), 'disabled observe should preserve stdin via runner passthrough'); + assert.ok(output.stderr.includes('shell runtime unavailable')); + }); + }); + })) passed++; else failed++; + + console.log(`\nPassed: ${passed}`); + console.log(`Failed: ${failed}`); + process.exit(failed > 0 ? 1 : 0); +} + +runTests();