From f37c92cfe25686a886b2692feeb38210687172b9 Mon Sep 17 00:00:00 2001 From: "to.watanabe" Date: Fri, 20 Mar 2026 16:00:17 +0900 Subject: [PATCH] fix(clv2): add --allowedTools to observer Haiku invocation (#661) The observer's Haiku subprocess cannot access files outside the project sandbox (/tmp/ for observations, ~/.claude/homunculus/ for instincts). Adding --allowedTools "Read,Write" grants the necessary file access while keeping the subprocess constrained by --max-turns and timeout. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../agents/observer-loop.sh | 4 +- tests/hooks/observer-memory.test.js | 159 ++++++++---------- 2 files changed, 73 insertions(+), 90 deletions(-) diff --git a/skills/continuous-learning-v2/agents/observer-loop.sh b/skills/continuous-learning-v2/agents/observer-loop.sh index 2be2049a..0d54070b 100755 --- a/skills/continuous-learning-v2/agents/observer-loop.sh +++ b/skills/continuous-learning-v2/agents/observer-loop.sh @@ -114,7 +114,9 @@ PROMPT fi # Prevent observe.sh from recording this automated Haiku session as observations - ECC_SKIP_OBSERVE=1 ECC_HOOK_PROFILE=minimal claude --model haiku --max-turns "$max_turns" --print < "$prompt_file" >> "$LOG_FILE" 2>&1 & + ECC_SKIP_OBSERVE=1 ECC_HOOK_PROFILE=minimal claude --model haiku --max-turns "$max_turns" --print \ + --allowedTools "Read,Write" \ + < "$prompt_file" >> "$LOG_FILE" 2>&1 & claude_pid=$! ( diff --git a/tests/hooks/observer-memory.test.js b/tests/hooks/observer-memory.test.js index b380386c..c2147f26 100644 --- a/tests/hooks/observer-memory.test.js +++ b/tests/hooks/observer-memory.test.js @@ -56,42 +56,24 @@ console.log('--- observe.sh signal throttling ---'); test('observe.sh contains SIGNAL_EVERY_N throttle variable', () => { const content = fs.readFileSync(observeShPath, 'utf8'); - assert.ok( - content.includes('SIGNAL_EVERY_N'), - 'observe.sh should define SIGNAL_EVERY_N for throttling' - ); + assert.ok(content.includes('SIGNAL_EVERY_N'), 'observe.sh should define SIGNAL_EVERY_N for throttling'); }); test('observe.sh uses a counter file instead of signaling every call', () => { const content = fs.readFileSync(observeShPath, 'utf8'); - assert.ok( - content.includes('.observer-signal-counter'), - 'observe.sh should use a signal counter file' - ); + assert.ok(content.includes('.observer-signal-counter'), 'observe.sh should use a signal counter file'); }); test('observe.sh only signals when counter reaches threshold', () => { const content = fs.readFileSync(observeShPath, 'utf8'); - assert.ok( - content.includes('should_signal=0'), - 'observe.sh should default should_signal to 0' - ); - assert.ok( - content.includes('should_signal=1'), - 'observe.sh should set should_signal=1 when threshold reached' - ); - assert.ok( - content.includes('if [ "$should_signal" -eq 1 ]'), - 'observe.sh should gate kill -USR1 behind should_signal check' - ); + assert.ok(content.includes('should_signal=0'), 'observe.sh should default should_signal to 0'); + assert.ok(content.includes('should_signal=1'), 'observe.sh should set should_signal=1 when threshold reached'); + assert.ok(content.includes('if [ "$should_signal" -eq 1 ]'), 'observe.sh should gate kill -USR1 behind should_signal check'); }); test('observe.sh default throttle is 20 observations per signal', () => { const content = fs.readFileSync(observeShPath, 'utf8'); - assert.ok( - content.includes('ECC_OBSERVER_SIGNAL_EVERY_N:-20'), - 'Default signal frequency should be every 20 observations' - ); + assert.ok(content.includes('ECC_OBSERVER_SIGNAL_EVERY_N:-20'), 'Default signal frequency should be every 20 observations'); }); // ────────────────────────────────────────────────────── @@ -102,22 +84,13 @@ console.log('\n--- observer-loop.sh re-entrancy guard ---'); test('observer-loop.sh defines ANALYZING guard variable', () => { const content = fs.readFileSync(observerLoopPath, 'utf8'); - assert.ok( - content.includes('ANALYZING=0'), - 'observer-loop.sh should initialize ANALYZING=0' - ); + assert.ok(content.includes('ANALYZING=0'), 'observer-loop.sh should initialize ANALYZING=0'); }); test('on_usr1 checks ANALYZING before starting analysis', () => { const content = fs.readFileSync(observerLoopPath, 'utf8'); - assert.ok( - content.includes('if [ "$ANALYZING" -eq 1 ]'), - 'on_usr1 should check ANALYZING flag' - ); - assert.ok( - content.includes('Analysis already in progress, skipping signal'), - 'on_usr1 should log when skipping due to re-entrancy' - ); + assert.ok(content.includes('if [ "$ANALYZING" -eq 1 ]'), 'on_usr1 should check ANALYZING flag'); + assert.ok(content.includes('Analysis already in progress, skipping signal'), 'on_usr1 should log when skipping due to re-entrancy'); }); test('on_usr1 sets ANALYZING=1 before and ANALYZING=0 after analysis', () => { @@ -139,30 +112,18 @@ console.log('\n--- observer-loop.sh cooldown throttle ---'); test('observer-loop.sh defines ANALYSIS_COOLDOWN', () => { const content = fs.readFileSync(observerLoopPath, 'utf8'); - assert.ok( - content.includes('ANALYSIS_COOLDOWN'), - 'observer-loop.sh should define ANALYSIS_COOLDOWN' - ); + assert.ok(content.includes('ANALYSIS_COOLDOWN'), 'observer-loop.sh should define ANALYSIS_COOLDOWN'); }); test('on_usr1 enforces cooldown between analyses', () => { const content = fs.readFileSync(observerLoopPath, 'utf8'); - assert.ok( - content.includes('LAST_ANALYSIS_EPOCH'), - 'Should track last analysis time' - ); - assert.ok( - content.includes('Analysis cooldown active'), - 'Should log when cooldown prevents analysis' - ); + assert.ok(content.includes('LAST_ANALYSIS_EPOCH'), 'Should track last analysis time'); + assert.ok(content.includes('Analysis cooldown active'), 'Should log when cooldown prevents analysis'); }); test('default cooldown is 60 seconds', () => { const content = fs.readFileSync(observerLoopPath, 'utf8'); - assert.ok( - content.includes('ECC_OBSERVER_ANALYSIS_COOLDOWN:-60'), - 'Default cooldown should be 60 seconds' - ); + assert.ok(content.includes('ECC_OBSERVER_ANALYSIS_COOLDOWN:-60'), 'Default cooldown should be 60 seconds'); }); // ────────────────────────────────────────────────────── @@ -173,30 +134,18 @@ console.log('\n--- observer-loop.sh tail-based sampling ---'); test('analyze_observations uses tail to sample recent observations', () => { const content = fs.readFileSync(observerLoopPath, 'utf8'); - assert.ok( - content.includes('tail -n "$MAX_ANALYSIS_LINES"'), - 'Should use tail to limit observations sent to LLM' - ); + assert.ok(content.includes('tail -n "$MAX_ANALYSIS_LINES"'), 'Should use tail to limit observations sent to LLM'); }); test('default max analysis lines is 500', () => { const content = fs.readFileSync(observerLoopPath, 'utf8'); - assert.ok( - content.includes('ECC_OBSERVER_MAX_ANALYSIS_LINES:-500'), - 'Default should sample last 500 lines' - ); + assert.ok(content.includes('ECC_OBSERVER_MAX_ANALYSIS_LINES:-500'), 'Default should sample last 500 lines'); }); test('analysis temp file is created and cleaned up', () => { const content = fs.readFileSync(observerLoopPath, 'utf8'); - assert.ok( - content.includes('ecc-observer-analysis'), - 'Should create a temp analysis file' - ); - assert.ok( - content.includes('rm -f "$prompt_file" "$analysis_file"'), - 'Should clean up both prompt and analysis temp files' - ); + assert.ok(content.includes('ecc-observer-analysis'), 'Should create a temp analysis file'); + assert.ok(content.includes('rm -f "$prompt_file" "$analysis_file"'), 'Should clean up both prompt and analysis temp files'); }); test('prompt references analysis_file not full OBSERVATIONS_FILE', () => { @@ -208,10 +157,7 @@ test('prompt references analysis_file not full OBSERVATIONS_FILE', () => { assert.ok(heredocStart > 0, 'Should find prompt heredoc start'); assert.ok(heredocEnd > heredocStart, 'Should find prompt heredoc end'); const promptSection = content.substring(heredocStart, heredocEnd); - assert.ok( - promptSection.includes('${analysis_file}'), - 'Prompt should point Claude at the sampled analysis file, not the full observations file' - ); + assert.ok(promptSection.includes('${analysis_file}'), 'Prompt should point Claude at the sampled analysis file, not the full observations file'); }); // ────────────────────────────────────────────────────── @@ -287,22 +233,22 @@ test('observe.sh creates counter file and increments on each call', () => { fs.mkdirSync(hooksDir, { recursive: true }); // Minimal detect-project.sh stub - fs.writeFileSync(path.join(scriptsDir, 'detect-project.sh'), [ - '#!/bin/bash', - `PROJECT_ID="test-project"`, - `PROJECT_NAME="test-project"`, - `PROJECT_ROOT="${projectDir}"`, - `PROJECT_DIR="${projectDir}"`, - `CLV2_PYTHON_CMD="${process.platform === 'win32' ? 'python' : 'python3'}"`, - '' - ].join('\n')); + fs.writeFileSync( + path.join(scriptsDir, 'detect-project.sh'), + [ + '#!/bin/bash', + `PROJECT_ID="test-project"`, + `PROJECT_NAME="test-project"`, + `PROJECT_ROOT="${projectDir}"`, + `PROJECT_DIR="${projectDir}"`, + `CLV2_PYTHON_CMD="${process.platform === 'win32' ? 'python' : 'python3'}"`, + '' + ].join('\n') + ); // Copy observe.sh but patch SKILL_ROOT to our test dir let observeContent = fs.readFileSync(observeShPath, 'utf8'); - observeContent = observeContent.replace( - 'SKILL_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"', - `SKILL_ROOT="${skillRoot}"` - ); + observeContent = observeContent.replace('SKILL_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"', `SKILL_ROOT="${skillRoot}"`); const testObserve = path.join(hooksDir, 'observe.sh'); fs.writeFileSync(testObserve, observeContent, { mode: 0o755 }); @@ -333,10 +279,7 @@ test('observe.sh creates counter file and increments on each call', () => { if (fs.existsSync(counterFile)) { const val = fs.readFileSync(counterFile, 'utf8').trim(); const counterVal = parseInt(val, 10); - assert.ok( - counterVal >= 1 && counterVal <= 2, - `Counter should be 1 or 2 after 2 calls, got ${counterVal}` - ); + assert.ok(counterVal >= 1 && counterVal <= 2, `Counter should be 1 or 2 after 2 calls, got ${counterVal}`); } else { // If python3 is not available the hook exits early - that is acceptable const hasPython = spawnSync('python3', ['--version']).status === 0; @@ -348,6 +291,44 @@ test('observe.sh creates counter file and increments on each call', () => { cleanupDir(testDir); }); +// ────────────────────────────────────────────────────── +// Test group 7: Observer Haiku invocation flags +// ────────────────────────────────────────────────────── + +console.log('\n--- Observer Haiku invocation flags ---'); + +test('claude invocation includes --allowedTools flag', () => { + const content = fs.readFileSync(observerLoopPath, 'utf8'); + assert.ok(content.includes('--allowedTools'), 'observer-loop.sh should include --allowedTools flag in claude invocation'); +}); + +test('allowedTools includes Read permission', () => { + const content = fs.readFileSync(observerLoopPath, 'utf8'); + const match = content.match(/--allowedTools\s+"([^"]+)"/); + assert.ok(match, 'Should find --allowedTools with quoted value'); + assert.ok(match[1].includes('Read'), `allowedTools should include Read, got: ${match[1]}`); +}); + +test('allowedTools includes Write permission', () => { + const content = fs.readFileSync(observerLoopPath, 'utf8'); + const match = content.match(/--allowedTools\s+"([^"]+)"/); + assert.ok(match, 'Should find --allowedTools with quoted value'); + assert.ok(match[1].includes('Write'), `allowedTools should include Write, got: ${match[1]}`); +}); + +test('claude invocation still includes ECC_SKIP_OBSERVE and ECC_HOOK_PROFILE guards', () => { + const content = fs.readFileSync(observerLoopPath, 'utf8'); + // Find the claude execution line(s) + const lines = content.split('\n'); + const claudeLine = lines.find(l => l.includes('claude --model haiku')); + assert.ok(claudeLine, 'Should find claude --model haiku invocation line'); + // The env vars are on the same line as the claude command + const claudeLineIndex = lines.indexOf(claudeLine); + const fullCommand = lines.slice(Math.max(0, claudeLineIndex - 1), claudeLineIndex + 3).join(' '); + assert.ok(fullCommand.includes('ECC_SKIP_OBSERVE=1'), 'claude invocation should include ECC_SKIP_OBSERVE=1 guard'); + assert.ok(fullCommand.includes('ECC_HOOK_PROFILE=minimal'), 'claude invocation should include ECC_HOOK_PROFILE=minimal guard'); +}); + // ────────────────────────────────────────────────────── // Summary // ──────────────────────────────────────────────────────