fix(observer): C5 coverage driven by hook registration, drop commit ratio (COV-1)
Bug: checkCoverage flagged anomaly when "recent commits > 0 AND episodes == 0". Two design flaws, proven in this project: - Wrong unit: commits = work-unit (one turn → many commits via subagent workflow); episodes = turn-unit. A 1023-vs-19 ratio is not anomalous, it's expected. - Wrong window: the 14-day commit window predated the Stop-hook's existence (registered 2026-05-19). For 13 of 14 days the hook didn't exist — 889 commits were structurally impossible to mirror as episodes. Result: the C5 indicator was either always-red (flagging the hook's birth as anomaly) or always-green (any episode count vs huge commit count = ok). Either way uninformative. Fix: - checkCoverage(episodeCount, hookRegistered) — drops the commit param. Warn iff hook is registered AND 0 episodes this month → the hook is silently failing. If the hook isn't registered, 0 episodes is correct. - runCoverageChecker derives hookRegistered from settings.json (isObserverStopRegistered helper) and passes it to checkCoverage. No more git execFileSync — pure fs. Tests rewritten under the new contract: 7/7 (was 6, +1 drift-hazard guard ensuring detail strings never mention "commit"). 15/15 coverage tests green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -2,30 +2,35 @@
|
||||
/**
|
||||
* C5 observer-coverage-checker (brain governance, observer factor-analysis
|
||||
* spec §5.2). Warn-only — always exits 0. Two checks:
|
||||
* 1. Coverage — recent git commits but 0 observer episodes this month.
|
||||
* 1. Coverage — Stop-hook is registered but 0 episodes this month.
|
||||
* Comparing episodes against commit volume is wrong-unit (commits =
|
||||
* work-unit, episodes = turn-unit) and wrong-window (the C5 window
|
||||
* can predate the hook's registration); a freshly-registered hook
|
||||
* vs. 1000 historical commits would flap forever. Driven by hook
|
||||
* registration instead — the only honest expectation source.
|
||||
* 2. Registration integrity — observer Stop-hook present in
|
||||
* .claude/settings.json and .git/hooks/post-commit installed.
|
||||
* Findings are surfaced in docs/observer/STATUS.md (C4 generator); this
|
||||
* controller never blocks a commit.
|
||||
*
|
||||
* Security Guidance #40: git is invoked via execFileSync (argument array,
|
||||
* no shell) — no exec/execSync.
|
||||
* Security Guidance #40: pure fs — no exec/execSync.
|
||||
*/
|
||||
import { readFileSync, existsSync } from 'fs';
|
||||
import { join } from 'path';
|
||||
import { execFileSync } from 'child_process';
|
||||
|
||||
const RECENT_WINDOW = '14 days ago';
|
||||
|
||||
/** @returns {{ok: boolean, detail: string}} */
|
||||
export function checkCoverage(episodeCount, recentCommitCount) {
|
||||
if (recentCommitCount > 0 && episodeCount === 0) {
|
||||
/**
|
||||
* @param {number} episodeCount - episodes in the current month JSONL
|
||||
* @param {boolean} hookRegistered - whether observer-stop-hook is wired in settings
|
||||
* @returns {{ok: boolean, detail: string}}
|
||||
*/
|
||||
export function checkCoverage(episodeCount, hookRegistered) {
|
||||
if (hookRegistered && episodeCount === 0) {
|
||||
return {
|
||||
ok: false,
|
||||
detail: `${recentCommitCount} commit(s) in the last 2 weeks but 0 observer episodes this month`,
|
||||
detail: `Stop-hook registered but 0 episode(s) recorded this month — hook may be silently failing`,
|
||||
};
|
||||
}
|
||||
return { ok: true, detail: `${episodeCount} episode(s), ${recentCommitCount} recent commit(s)` };
|
||||
return { ok: true, detail: `${episodeCount} episode(s) this month` };
|
||||
}
|
||||
|
||||
/** @returns {{ok: boolean, detail: string}} */
|
||||
@@ -54,27 +59,27 @@ function countEpisodes(root) {
|
||||
return readFileSync(file, 'utf-8').trim().split('\n').filter(Boolean).length;
|
||||
}
|
||||
|
||||
function countRecentCommits(root) {
|
||||
function readSettings(root) {
|
||||
try {
|
||||
const out = execFileSync('git', ['log', `--since=${RECENT_WINDOW}`, '--oneline'], {
|
||||
cwd: root,
|
||||
encoding: 'utf-8',
|
||||
stdio: ['ignore', 'pipe', 'ignore'],
|
||||
});
|
||||
return out.trim() ? out.trim().split('\n').length : 0;
|
||||
return JSON.parse(readFileSync(join(root, '.claude', 'settings.json'), 'utf-8'));
|
||||
} catch {
|
||||
return 0;
|
||||
return {};
|
||||
}
|
||||
}
|
||||
|
||||
function isObserverStopRegistered(settings) {
|
||||
const stopHooks = (((settings || {}).hooks || {}).Stop) || [];
|
||||
return stopHooks.some((entry) =>
|
||||
((entry && entry.hooks) || []).some((h) =>
|
||||
String((h && h.command) || '').includes('observer-stop-hook')
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
export function runCoverageChecker(root = process.cwd()) {
|
||||
const coverage = checkCoverage(countEpisodes(root), countRecentCommits(root));
|
||||
let settings = {};
|
||||
try {
|
||||
settings = JSON.parse(readFileSync(join(root, '.claude', 'settings.json'), 'utf-8'));
|
||||
} catch {
|
||||
settings = {};
|
||||
}
|
||||
const settings = readSettings(root);
|
||||
const hookRegistered = isObserverStopRegistered(settings);
|
||||
const coverage = checkCoverage(countEpisodes(root), hookRegistered);
|
||||
const registration = checkRegistration(settings, existsSync(join(root, '.git', 'hooks', 'post-commit')));
|
||||
return { coverage, registration };
|
||||
}
|
||||
|
||||
@@ -2,18 +2,34 @@ import { describe, it, expect } from 'vitest';
|
||||
import { checkCoverage, checkRegistration } from './observer-coverage-checker.mjs';
|
||||
|
||||
describe('checkCoverage', () => {
|
||||
it('flags recent commits but zero episodes', () => {
|
||||
const r = checkCoverage(0, 7);
|
||||
// COV-1 fix: the metric is driven by Stop-hook registration, NOT by recent
|
||||
// commit volume. Comparing commits-to-episodes is wrong-unit + wrong-window
|
||||
// (commits ≠ turns; a 14-day window mostly predates a freshly-registered
|
||||
// hook). The honest signal is: "the hook is registered, so we expect
|
||||
// episodes — if there are 0 this month, the hook is silently dead".
|
||||
|
||||
it('flags 0 episodes when the Stop-hook is registered', () => {
|
||||
const r = checkCoverage(0, true);
|
||||
expect(r.ok).toBe(false);
|
||||
expect(r.detail).toContain('0 observer episodes');
|
||||
expect(r.detail).toContain('0');
|
||||
expect(r.detail).toMatch(/registered|episode/i);
|
||||
});
|
||||
|
||||
it('is ok when episodes exist', () => {
|
||||
expect(checkCoverage(5, 7).ok).toBe(true);
|
||||
it('is ok when episodes exist and hook is registered', () => {
|
||||
expect(checkCoverage(5, true).ok).toBe(true);
|
||||
});
|
||||
|
||||
it('is ok when there is no recent git activity', () => {
|
||||
expect(checkCoverage(0, 0).ok).toBe(true);
|
||||
it('is ok when the hook is NOT registered (no expectation of episodes)', () => {
|
||||
// If the hook was never installed in this repo, 0 episodes is correct,
|
||||
// not a defect — silence here would have been a false alarm.
|
||||
expect(checkCoverage(0, false).ok).toBe(true);
|
||||
});
|
||||
|
||||
it('detail does NOT reference a commit-count ratio (drift hazard)', () => {
|
||||
// The legacy "X episodes vs Y commits" wording implied commit≈episode is
|
||||
// a target — misleading because commits=work-unit, episodes=turn-unit.
|
||||
expect(checkCoverage(5, true).detail).not.toMatch(/commit/i);
|
||||
expect(checkCoverage(0, true).detail).not.toMatch(/commit/i);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user