From 9af6606142b88311fc543496567542fedab45042 Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Tue, 25 Aug 2026 17:12:08 +0700 Subject: [PATCH 1/3] fix(continuous-learning-v2): count every instinct extension in observer status (#2859) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `start-observer.sh status` globbed `*.yaml`, but the observer prompt tells the analyzer to write `${INSTINCTS_DIR}/.md` and the loader accepts `.yaml`, `.yml`, and `.md` (ALLOWED_INSTINCT_EXTENSIONS in scripts/instinct-cli.py). The one command an operator runs to confirm that learning works therefore reported `Instincts: 0` on a healthy install — which is indistinguishable from a silently dead observer, exactly the failure the status check exists to surface. Match the loader instead of one of its three extensions, and match how it enumerates them: `Path.iterdir()` is top level only and `is_file()` skips directories, so `-maxdepth 1 -type f`; `suffix.lower()` makes the comparison case-insensitive, so `-iname`. `tr` drops the column padding BSD `wc` emits, which is why the reported output read `Instincts: 0`. Verified end to end against the shipped script with 3 `.md`, one `.yaml`, one `.yml`, one `.YAML`, a `notes.txt`, and a nested `.md`: 1 before, 6 after — the same six files the loader picks up. --- .../agents/start-observer.sh | 10 +- .../observer-status-instinct-count.test.js | 145 ++++++++++++++++++ 2 files changed, 153 insertions(+), 2 deletions(-) create mode 100644 tests/skills/observer-status-instinct-count.test.js diff --git a/skills/continuous-learning-v2/agents/start-observer.sh b/skills/continuous-learning-v2/agents/start-observer.sh index 5485a79e3f..5dc311b70e 100755 --- a/skills/continuous-learning-v2/agents/start-observer.sh +++ b/skills/continuous-learning-v2/agents/start-observer.sh @@ -156,8 +156,14 @@ case "$ACTION" in echo "Observer is running (PID: $pid)" echo "Log: $LOG_FILE" echo "Observations: $(wc -l < "$OBSERVATIONS_FILE" 2>/dev/null || echo 0) lines" - # Also show instinct count - instinct_count=$(find "$INSTINCTS_DIR" -name "*.yaml" 2>/dev/null | wc -l) + # Also show instinct count. Count every extension the loader accepts + # (ALLOWED_INSTINCT_EXTENSIONS in scripts/instinct-cli.py) - the + # observer prompt tells the analyzer to write ".md", so a + # *.yaml-only count reports 0 on a working install. Depth and case + # match the loader's iterdir() + suffix.lower(): top level only, + # case-insensitive. tr strips the padding BSD wc emits. + instinct_find_expr=( \( -iname "*.yaml" -o -iname "*.yml" -o -iname "*.md" \) ) + instinct_count=$(find "$INSTINCTS_DIR" -maxdepth 1 -type f "${instinct_find_expr[@]}" 2>/dev/null | wc -l | tr -d "[:space:]") echo "Instincts: $instinct_count" exit 0 else diff --git a/tests/skills/observer-status-instinct-count.test.js b/tests/skills/observer-status-instinct-count.test.js new file mode 100644 index 0000000000..420d66539c --- /dev/null +++ b/tests/skills/observer-status-instinct-count.test.js @@ -0,0 +1,145 @@ +/** + * Regression tests for #2859: `start-observer.sh status` counted only *.yaml. + * + * The producer writes `.md` (agents/observer-loop.sh instructs the analyzer + * to) and the loader accepts .yaml/.yml/.md (ALLOWED_INSTINCT_EXTENSIONS in + * scripts/instinct-cli.py), so the one command an operator runs to confirm that + * learning works reported `Instincts: 0` on a working install — and an operator + * cannot tell that apart from a silently dead observer. + */ + +'use strict'; + +const assert = require('assert'); +const { spawnSync } = require('child_process'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +const repoRoot = path.resolve(__dirname, '..', '..'); +const skillRoot = path.join(repoRoot, 'skills', 'continuous-learning-v2'); +const observerScript = path.join(skillRoot, 'agents', 'start-observer.sh'); +const detectProject = path.join(skillRoot, 'scripts', 'detect-project.sh'); +const instinctCli = path.join(skillRoot, 'scripts', 'instinct-cli.py'); +const bashBinary = process.env.ECC_TEST_BASH || (process.platform === 'win32' ? null : 'bash'); + +let passed = 0; + +function toShellPath(filePath) { + const normalized = filePath.split(path.sep).join('/'); + return normalized.replace(/^([A-Za-z]):\//, (_, drive) => `/${drive.toLowerCase()}/`); +} + +// ── The counter must accept every extension the loader accepts ── + +const cliSource = fs.readFileSync(instinctCli, 'utf8'); +const allowedMatch = cliSource.match(/ALLOWED_INSTINCT_EXTENSIONS\s*=\s*\(([^)]*)\)/); +assert.ok(allowedMatch, 'ALLOWED_INSTINCT_EXTENSIONS not found in instinct-cli.py'); +const allowedExtensions = allowedMatch[1] + .split(',') + .map(part => part.trim().replace(/^["']|["']$/g, '')) + .filter(Boolean); +assert.ok(allowedExtensions.length >= 3, `expected several extensions, got ${allowedExtensions}`); +passed++; + +const observerSource = fs.readFileSync(observerScript, 'utf8'); +const statusCount = observerSource + .split('\n') + .filter(line => line.includes('instinct_count=') || line.includes('instinct_find_expr=')) + .join('\n'); +assert.ok(statusCount, 'status branch no longer computes an instinct count'); + +for (const ext of allowedExtensions) { + assert.ok( + statusCount.includes(`*${ext}"`) || statusCount.includes(`*${ext}'`), + `status count must match ${ext} — the loader accepts it (ALLOWED_INSTINCT_EXTENSIONS)` + ); + passed++; +} + +// Depth and case must match the loader: Path.iterdir() is top-level only and +// is_file() skips directories; suffix.lower() makes the match case-insensitive. +assert.ok(statusCount.includes('-maxdepth 1'), 'status count must not recurse — the loader does not'); +assert.ok(statusCount.includes('-type f'), 'status count must skip directories'); +assert.ok(!/-name\s+["']\*/.test(statusCount), 'status count must use case-insensitive -iname'); +passed += 3; + +// ── The shipped script, run for real ── + +function resolvePython() { + for (const candidate of [process.env.ECC_TEST_PYTHON, 'python3', 'python']) { + if (!candidate) continue; + const probe = spawnSync(candidate, ['-c', 'print(1)'], { encoding: 'utf8' }); + if (probe.status === 0) return candidate; + } + return null; +} + +const pythonCmd = bashBinary ? resolvePython() : null; + +function runStatus(files) { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-observer-')); + try { + const projectDir = path.join(tmp, 'proj'); + fs.mkdirSync(projectDir, { recursive: true }); + const writes = files + .map(name => `printf 'id: x' > "$INST/${name}"`) + .join('\n '); + const script = [ + `source "${toShellPath(detectProject)}"`, + 'INST="$PROJECT_DIR/instincts/personal"', + 'mkdir -p "$INST/nested"', + writes, + ': > "$PROJECT_DIR/observations.jsonl"', + 'sleep 10 &', + 'OBSERVER_PID=$!', + 'echo "$OBSERVER_PID" > "$PROJECT_DIR/.observer.pid"', + `bash "${toShellPath(observerScript)}" status`, + 'status=$?', + 'kill "$OBSERVER_PID" 2>/dev/null || true', + 'exit $status', + ].join('\n'); + const result = spawnSync(bashBinary, ['-c', script], { + encoding: 'utf8', + env: { + ...process.env, + CLV2_HOMUNCULUS_DIR: toShellPath(path.join(tmp, 'homunculus')), + CLAUDE_PROJECT_DIR: toShellPath(projectDir), + CLV2_PYTHON_CMD: pythonCmd, + }, + }); + assert.strictEqual(result.status, 0, result.stderr || result.stdout); + const line = (result.stdout || '').split('\n').find(l => l.startsWith('Instincts:')); + assert.ok(line, `no "Instincts:" line in status output:\n${result.stdout}`); + return Number(line.split(':')[1].trim()); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } +} + +if (bashBinary && pythonCmd) { + const syntax = spawnSync(bashBinary, ['-n', toShellPath(observerScript)], { encoding: 'utf8' }); + assert.strictEqual(syntax.status, 0, syntax.stderr); + passed++; + + // The reported shape: every instinct on disk is a .md file. + assert.strictEqual(runStatus(['a.md', 'b.md', 'c.md']), 3, 'markdown instincts must be counted'); + passed++; + + // Every accepted extension, mixed case, plus the two things the loader skips: + // a non-instinct file and a nested directory. + assert.strictEqual( + runStatus(['a.md', 'b.yaml', 'c.yml', 'd.YAML', 'notes.txt', 'nested/deep.md']), + 4, + 'count must match the loader: every allowed extension, case-insensitive, top level only' + ); + passed++; + + assert.strictEqual(runStatus([]), 0, 'an empty instincts directory must still report 0'); + passed++; +} else { + console.log(' Integration coverage skipped (needs bash + python; set ECC_TEST_BASH/ECC_TEST_PYTHON)'); +} + +console.log(` Passed: ${passed}`); +console.log(' Failed: 0'); From ed6e5a42ee23a1ce9a2c60d78d993a2c413200cb Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Tue, 25 Aug 2026 18:03:35 +0700 Subject: [PATCH 2/3] test(observer-status): report every case, not just the first failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on #2878: the static assertions ran at the top level, so a failure exited the process before the `Passed:`/`Failed:` lines. tests/run-all.js totals those tokens, so the per-case counts were lost — it still went red via the non-zero exit, but the granular numbers were not in the totals. Route every case through the same `runTest()` wrapper the file already used for the integration cases, and build the case list inside a try so a missing or renamed `instinct-cli.py` / `start-observer.sh` is a reported failure rather than a crash. Reverting the fix now prints `Passed: 4, Failed: 7` and exits 1, naming all seven broken expectations instead of stopping at the first. --- .../observer-status-instinct-count.test.js | 173 ++++++++++++------ 1 file changed, 119 insertions(+), 54 deletions(-) diff --git a/tests/skills/observer-status-instinct-count.test.js b/tests/skills/observer-status-instinct-count.test.js index 420d66539c..223c40325e 100644 --- a/tests/skills/observer-status-instinct-count.test.js +++ b/tests/skills/observer-status-instinct-count.test.js @@ -23,48 +23,30 @@ const detectProject = path.join(skillRoot, 'scripts', 'detect-project.sh'); const instinctCli = path.join(skillRoot, 'scripts', 'instinct-cli.py'); const bashBinary = process.env.ECC_TEST_BASH || (process.platform === 'win32' ? null : 'bash'); -let passed = 0; - function toShellPath(filePath) { const normalized = filePath.split(path.sep).join('/'); return normalized.replace(/^([A-Za-z]):\//, (_, drive) => `/${drive.toLowerCase()}/`); } -// ── The counter must accept every extension the loader accepts ── - -const cliSource = fs.readFileSync(instinctCli, 'utf8'); -const allowedMatch = cliSource.match(/ALLOWED_INSTINCT_EXTENSIONS\s*=\s*\(([^)]*)\)/); -assert.ok(allowedMatch, 'ALLOWED_INSTINCT_EXTENSIONS not found in instinct-cli.py'); -const allowedExtensions = allowedMatch[1] - .split(',') - .map(part => part.trim().replace(/^["']|["']$/g, '')) - .filter(Boolean); -assert.ok(allowedExtensions.length >= 3, `expected several extensions, got ${allowedExtensions}`); -passed++; - -const observerSource = fs.readFileSync(observerScript, 'utf8'); -const statusCount = observerSource - .split('\n') - .filter(line => line.includes('instinct_count=') || line.includes('instinct_find_expr=')) - .join('\n'); -assert.ok(statusCount, 'status branch no longer computes an instinct count'); - -for (const ext of allowedExtensions) { - assert.ok( - statusCount.includes(`*${ext}"`) || statusCount.includes(`*${ext}'`), - `status count must match ${ext} — the loader accepts it (ALLOWED_INSTINCT_EXTENSIONS)` - ); - passed++; +function readAllowedExtensions() { + const cliSource = fs.readFileSync(instinctCli, 'utf8'); + const match = cliSource.match(/ALLOWED_INSTINCT_EXTENSIONS\s*=\s*\(([^)]*)\)/); + assert.ok(match, 'ALLOWED_INSTINCT_EXTENSIONS not found in instinct-cli.py'); + return match[1] + .split(',') + .map(part => part.trim().replace(/^["']|["']$/g, '')) + .filter(Boolean); } -// Depth and case must match the loader: Path.iterdir() is top-level only and -// is_file() skips directories; suffix.lower() makes the match case-insensitive. -assert.ok(statusCount.includes('-maxdepth 1'), 'status count must not recurse — the loader does not'); -assert.ok(statusCount.includes('-type f'), 'status count must skip directories'); -assert.ok(!/-name\s+["']\*/.test(statusCount), 'status count must use case-insensitive -iname'); -passed += 3; - -// ── The shipped script, run for real ── +function readStatusCounter() { + const observerSource = fs.readFileSync(observerScript, 'utf8'); + const counter = observerSource + .split('\n') + .filter(line => line.includes('instinct_count=') || line.includes('instinct_find_expr=')) + .join('\n'); + assert.ok(counter, 'status branch no longer computes an instinct count'); + return counter; +} function resolvePython() { for (const candidate of [process.env.ECC_TEST_PYTHON, 'python3', 'python']) { @@ -117,29 +99,112 @@ function runStatus(files) { } } -if (bashBinary && pythonCmd) { - const syntax = spawnSync(bashBinary, ['-n', toShellPath(observerScript)], { encoding: 'utf8' }); - assert.strictEqual(syntax.status, 0, syntax.stderr); - passed++; +function buildTests() { + const tests = []; + + // ── The counter must accept every extension the loader accepts ── + + tests.push(['the loader still declares several instinct extensions', () => { + const allowed = readAllowedExtensions(); + assert.ok(allowed.length >= 3, `expected several extensions, got ${allowed}`); + }]); + + for (const ext of readAllowedExtensions()) { + tests.push([`status counts ${ext} — the loader accepts it`, () => { + const counter = readStatusCounter(); + assert.ok( + counter.includes(`*${ext}"`) || counter.includes(`*${ext}'`), + `status count must match ${ext} (ALLOWED_INSTINCT_EXTENSIONS)` + ); + }]); + } + + // Depth and case must match the loader: Path.iterdir() is top-level only and + // is_file() skips directories; suffix.lower() makes the match case-insensitive. + tests.push(['status does not recurse — the loader does not', () => { + assert.ok(readStatusCounter().includes('-maxdepth 1')); + }]); + tests.push(['status skips directories', () => { + assert.ok(readStatusCounter().includes('-type f')); + }]); + tests.push(['status matches case-insensitively', () => { + assert.ok(!/-name\s+["']\*/.test(readStatusCounter()), + 'status count must use -iname, not -name'); + }]); + + // ── The shipped script, run for real ── + + if (!(bashBinary && pythonCmd)) return tests; + + tests.push(['start-observer.sh parses', () => { + const syntax = spawnSync(bashBinary, ['-n', toShellPath(observerScript)], { encoding: 'utf8' }); + assert.strictEqual(syntax.status, 0, syntax.stderr); + }]); // The reported shape: every instinct on disk is a .md file. - assert.strictEqual(runStatus(['a.md', 'b.md', 'c.md']), 3, 'markdown instincts must be counted'); - passed++; + tests.push(['markdown instincts are counted', () => { + assert.strictEqual(runStatus(['a.md', 'b.md', 'c.md']), 3); + }]); // Every accepted extension, mixed case, plus the two things the loader skips: // a non-instinct file and a nested directory. - assert.strictEqual( - runStatus(['a.md', 'b.yaml', 'c.yml', 'd.YAML', 'notes.txt', 'nested/deep.md']), - 4, - 'count must match the loader: every allowed extension, case-insensitive, top level only' - ); - passed++; - - assert.strictEqual(runStatus([]), 0, 'an empty instincts directory must still report 0'); - passed++; -} else { - console.log(' Integration coverage skipped (needs bash + python; set ECC_TEST_BASH/ECC_TEST_PYTHON)'); + tests.push(['the count matches the loader exactly', () => { + assert.strictEqual( + runStatus(['a.md', 'b.yaml', 'c.yml', 'd.YAML', 'notes.txt', 'nested/deep.md']), + 4 + ); + }]); + + tests.push(['an empty instincts directory reports 0', () => { + assert.strictEqual(runStatus([]), 0); + }]); + + return tests; +} + +function runTest(name, fn) { + try { + fn(); + console.log(` ✓ ${name}`); + return true; + } catch (error) { + console.log(` ✗ ${name}`); + console.error(` ${error.message}`); + return false; + } +} + +function main() { + console.log('\n=== Testing observer status instinct count (#2859) ===\n'); + + let passed = 0; + let failed = 0; + let tests; + + // Collecting the cases reads instinct-cli.py and start-observer.sh, so a + // missing or renamed file has to be reported as a failure rather than crash + // the process — tests/run-all.js totals the "Passed:"/"Failed:" tokens below. + try { + tests = buildTests(); + } catch (error) { + console.log(' ✗ could not build the test list'); + console.error(` ${error.message}`); + tests = []; + failed += 1; + } + + for (const [name, fn] of tests) { + if (runTest(name, fn)) passed += 1; + else failed += 1; + } + + if (!(bashBinary && pythonCmd)) { + console.log(' - integration coverage skipped (needs bash + python; set ECC_TEST_BASH/ECC_TEST_PYTHON)'); + } + + console.log(`\n Passed: ${passed}`); + console.log(` Failed: ${failed}`); + if (failed > 0) process.exit(1); } -console.log(` Passed: ${passed}`); -console.log(' Failed: 0'); +main(); From e3ab97915fc77f2f368f015195d9fce8bae1a9fe Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Tue, 25 Aug 2026 18:37:00 +0700 Subject: [PATCH 3/3] test(observer-status): set process.exitCode so the summary always flushes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on #2878. stdout is async when it is a pipe, which is exactly how tests/run-all.js runs these files, and process.exit() does not wait for pending writes — so exiting that way can drop the Passed:/Failed: lines the aggregator totals, defeating the previous commit. The sibling tests/ci/ito-*-skill.test.js files already use process.exitCode. Still exits 1 on a broken counter (Passed: 4, Failed: 7) and 0 when clean. --- tests/skills/observer-status-instinct-count.test.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/skills/observer-status-instinct-count.test.js b/tests/skills/observer-status-instinct-count.test.js index 223c40325e..d65a850cae 100644 --- a/tests/skills/observer-status-instinct-count.test.js +++ b/tests/skills/observer-status-instinct-count.test.js @@ -204,7 +204,10 @@ function main() { console.log(`\n Passed: ${passed}`); console.log(` Failed: ${failed}`); - if (failed > 0) process.exit(1); + // exitCode, not exit(1): stdout is async when it is a pipe, which is how + // tests/run-all.js runs this, and process.exit() does not wait for pending + // writes — it could drop the two lines above, which the aggregator totals. + if (failed > 0) process.exitCode = 1; } main();