|
| 1 | +/** |
| 2 | + * Tests for observer-loop archive-on-failure fix (#2370) |
| 3 | + * |
| 4 | + * Bug: analyze_observations() in observer-loop.sh moved the live |
| 5 | + * observations.jsonl into observations.archive/ unconditionally, even when |
| 6 | + * the Claude analysis step failed (timeout, non-zero exit, rate limit). |
| 7 | + * Because the analyzer only ever reads the live file, a failed batch could |
| 8 | + * never be re-analyzed and its instincts were silently lost. |
| 9 | + * |
| 10 | + * Fix: archive only after a successful analysis; on failure log and return, |
| 11 | + * retaining observations for the next cycle to retry. |
| 12 | + * |
| 13 | + * Strategy: source observer-loop.sh (a BASH_SOURCE guard stops the main |
| 14 | + * loop from running when sourced) and drive analyze_observations directly |
| 15 | + * with a stub `claude` (exit code controlled per case) and a stub sibling |
| 16 | + * session-guardian.sh. Assert symmetric outcomes for failure vs success. |
| 17 | + * |
| 18 | + * Run with: node tests/hooks/observer-loop-archive.test.js |
| 19 | + */ |
| 20 | + |
| 21 | +const assert = require('assert'); |
| 22 | +const path = require('path'); |
| 23 | +const fs = require('fs'); |
| 24 | +const os = require('os'); |
| 25 | +const { spawnSync } = require('child_process'); |
| 26 | + |
| 27 | +let passed = 0; |
| 28 | +let failed = 0; |
| 29 | + |
| 30 | +function test(name, fn) { |
| 31 | + try { |
| 32 | + fn(); |
| 33 | + console.log(` ✓ ${name}`); |
| 34 | + passed++; |
| 35 | + } catch (err) { |
| 36 | + console.log(` ✗ ${name}`); |
| 37 | + console.log(` Error: ${err.message}`); |
| 38 | + failed++; |
| 39 | + } |
| 40 | +} |
| 41 | + |
| 42 | +function createTempDir() { |
| 43 | + return fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-observer-archive-')); |
| 44 | +} |
| 45 | + |
| 46 | +function cleanupDir(dir) { |
| 47 | + try { |
| 48 | + fs.rmSync(dir, { recursive: true, force: true }); |
| 49 | + } catch { |
| 50 | + // ignore cleanup errors |
| 51 | + } |
| 52 | +} |
| 53 | + |
| 54 | +const repoRoot = path.resolve(__dirname, '..', '..'); |
| 55 | +const observerLoopPath = path.join( |
| 56 | + repoRoot, 'skills', 'continuous-learning-v2', 'agents', 'observer-loop.sh' |
| 57 | +); |
| 58 | + |
| 59 | +/** |
| 60 | + * Run analyze_observations once with the given stub claude exit code. |
| 61 | + * Returns { liveExists, archivedCount, log } describing the resulting state. |
| 62 | + */ |
| 63 | +function runAnalyzeOnce(claudeExitCode) { |
| 64 | + const sandbox = createTempDir(); |
| 65 | + try { |
| 66 | + const binDir = path.join(sandbox, 'bin'); |
| 67 | + const projectDir = path.join(sandbox, 'project'); |
| 68 | + fs.mkdirSync(binDir, { recursive: true }); |
| 69 | + fs.mkdirSync(projectDir, { recursive: true }); |
| 70 | + |
| 71 | + // Stub claude: exit with the requested code, ignoring all args. |
| 72 | + const claudeStub = path.join(binDir, 'claude'); |
| 73 | + fs.writeFileSync(claudeStub, '#!/usr/bin/env bash\nexit ${CLAUDE_STUB_EXIT:-0}\n'); |
| 74 | + fs.chmodSync(claudeStub, 0o755); |
| 75 | + |
| 76 | + // analyze_observations resolves the real session-guardian.sh via its own |
| 77 | + // ${BASH_SOURCE[0]}-derived SCRIPT_DIR, so we drive the real guardian with |
| 78 | + // all of its gates disabled/isolated (see env below) rather than stubbing it. |
| 79 | + |
| 80 | + // Driver sources observer-loop.sh (guard stops the main loop) then runs |
| 81 | + // the single function under test. |
| 82 | + const driver = path.join(sandbox, 'driver.sh'); |
| 83 | + fs.writeFileSync( |
| 84 | + driver, |
| 85 | + `#!/usr/bin/env bash\nsource ${JSON.stringify(observerLoopPath)}\nanalyze_observations\n` |
| 86 | + ); |
| 87 | + fs.chmodSync(driver, 0o755); |
| 88 | + |
| 89 | + const observationsFile = path.join(projectDir, 'observations.jsonl'); |
| 90 | + fs.writeFileSync(observationsFile, '{"a":1}\n{"a":2}\n{"a":3}\n'); |
| 91 | + |
| 92 | + // Defensive: never leak CLAUDE_PLUGIN_ROOT into the ECC test shell (it |
| 93 | + // contaminates this project's hook-root resolution). |
| 94 | + const childEnv = Object.assign({}, process.env); |
| 95 | + delete childEnv.CLAUDE_PLUGIN_ROOT; |
| 96 | + childEnv.PATH = binDir + path.delimiter + process.env.PATH; |
| 97 | + childEnv.CLAUDE_STUB_EXIT = String(claudeExitCode); |
| 98 | + childEnv.OBSERVATIONS_FILE = observationsFile; |
| 99 | + childEnv.MIN_OBSERVATIONS = '1'; |
| 100 | + childEnv.PROJECT_DIR = projectDir; |
| 101 | + childEnv.LOG_FILE = path.join(projectDir, 'observer.log'); |
| 102 | + childEnv.PROJECT_NAME = 'test-project'; |
| 103 | + childEnv.PROJECT_ID = 'test-project'; |
| 104 | + childEnv.INSTINCTS_DIR = path.join(projectDir, 'instincts'); |
| 105 | + childEnv.CONFIG_DIR = projectDir; |
| 106 | + childEnv.CLV2_IS_WINDOWS = 'false'; |
| 107 | + childEnv.ECC_OBSERVER_TIMEOUT_SECONDS = '2'; |
| 108 | + // Make the real session-guardian.sh deterministically proceed (exit 0): |
| 109 | + // disable the active-hours and idle gates, isolate the cooldown log, and |
| 110 | + // zero the cooldown interval so a fresh project always passes. |
| 111 | + childEnv.OBSERVER_ACTIVE_HOURS_START = '0'; |
| 112 | + childEnv.OBSERVER_ACTIVE_HOURS_END = '0'; |
| 113 | + childEnv.OBSERVER_MAX_IDLE_SECONDS = '0'; |
| 114 | + childEnv.OBSERVER_INTERVAL_SECONDS = '0'; |
| 115 | + childEnv.OBSERVER_LAST_RUN_LOG = path.join(projectDir, 'observer-last-run.log'); |
| 116 | + |
| 117 | + const result = spawnSync('bash', [driver], { |
| 118 | + encoding: 'utf8', |
| 119 | + timeout: 15000, |
| 120 | + env: childEnv |
| 121 | + }); |
| 122 | + |
| 123 | + assert.strictEqual( |
| 124 | + result.status, 0, |
| 125 | + `driver should exit 0, got ${result.status}; stderr: ${result.stderr}` |
| 126 | + ); |
| 127 | + |
| 128 | + const archiveDir = path.join(projectDir, 'observations.archive'); |
| 129 | + let archivedCount = 0; |
| 130 | + if (fs.existsSync(archiveDir)) { |
| 131 | + archivedCount = fs.readdirSync(archiveDir) |
| 132 | + .filter(f => /^processed-.*\.jsonl$/.test(f)).length; |
| 133 | + } |
| 134 | + let log = ''; |
| 135 | + try { log = fs.readFileSync(childEnv.LOG_FILE, 'utf8'); } catch { /* none */ } |
| 136 | + |
| 137 | + return { liveExists: fs.existsSync(observationsFile), archivedCount, log }; |
| 138 | + } finally { |
| 139 | + cleanupDir(sandbox); |
| 140 | + } |
| 141 | +} |
| 142 | + |
| 143 | +console.log('\n=== Observer-loop Archive-on-Failure Tests (#2370) ===\n'); |
| 144 | + |
| 145 | +console.log('--- behavioral ---'); |
| 146 | + |
| 147 | +test('failed analysis retains observations and archives nothing', () => { |
| 148 | + // Shell-driven behavioral check; skip on Windows where the bash driver's |
| 149 | + // $0 path handling differs (matches observer-memory.test.js convention). |
| 150 | + if (process.platform === 'win32') { |
| 151 | + return; |
| 152 | + } |
| 153 | + const { liveExists, archivedCount, log } = runAnalyzeOnce(1); |
| 154 | + assert.ok(liveExists, 'live observations.jsonl must be retained when analysis fails'); |
| 155 | + assert.strictEqual(archivedCount, 0, 'nothing should be archived when analysis fails'); |
| 156 | + assert.ok( |
| 157 | + /retaining observations for retry/.test(log), |
| 158 | + `failure log should note retention; got: ${log}` |
| 159 | + ); |
| 160 | +}); |
| 161 | + |
| 162 | +test('successful analysis archives the batch (happy path preserved)', () => { |
| 163 | + // Shell-driven behavioral check; skip on Windows (see note above). |
| 164 | + if (process.platform === 'win32') { |
| 165 | + return; |
| 166 | + } |
| 167 | + const { liveExists, archivedCount } = runAnalyzeOnce(0); |
| 168 | + assert.ok(!liveExists, 'live observations.jsonl should be moved after a successful analysis'); |
| 169 | + assert.strictEqual(archivedCount, 1, 'exactly one processed-*.jsonl should be archived on success'); |
| 170 | +}); |
| 171 | + |
| 172 | +console.log('--- static guards ---'); |
| 173 | + |
| 174 | +test('analyze_observations returns on failure before the archive mv', () => { |
| 175 | + const content = fs.readFileSync(observerLoopPath, 'utf8'); |
| 176 | + // Operate on full file content with explicit anchors rather than a lazy |
| 177 | + // function-body extraction (which could truncate on a future inner "\n}" |
| 178 | + // and pass vacuously). These tokens each occur once, inside the function. |
| 179 | + const failIdx = content.search(/exit_code"?\s+-ne\s+0/); |
| 180 | + const returnIdx = content.indexOf('return', failIdx); |
| 181 | + const archiveIdx = content.indexOf('observations.archive'); |
| 182 | + assert.ok(failIdx !== -1, 'should find the non-zero exit_code check'); |
| 183 | + assert.ok(archiveIdx !== -1, 'should find the archive block'); |
| 184 | + assert.ok(returnIdx !== -1, 'failure branch should contain a return'); |
| 185 | + assert.ok(returnIdx < archiveIdx, |
| 186 | + 'failure branch must return before reaching the archive block'); |
| 187 | +}); |
| 188 | + |
| 189 | +test('observer-loop.sh has a source-guard so it can be sourced in tests', () => { |
| 190 | + const content = fs.readFileSync(observerLoopPath, 'utf8'); |
| 191 | + assert.ok( |
| 192 | + content.includes('BASH_SOURCE[0]') && content.includes('return 0 2>/dev/null'), |
| 193 | + 'observer-loop.sh should short-circuit when sourced rather than executed' |
| 194 | + ); |
| 195 | +}); |
| 196 | + |
| 197 | +console.log('\n=== Test Results ==='); |
| 198 | +console.log(`Passed: ${passed}`); |
| 199 | +console.log(`Failed: ${failed}`); |
| 200 | +console.log(`Total: ${passed + failed}\n`); |
| 201 | + |
| 202 | +process.exit(failed > 0 ? 1 : 0); |
0 commit comments