From 7fa0747961fdc19a74cdcf352e3f19e453695f2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=94=D0=BC=D0=B8=D1=82=D1=80=D0=B8=D0=B9?= Date: Wed, 20 May 2026 11:07:58 +0300 Subject: [PATCH] fix(observer): C5 coverage driven by hook registration, drop commit ratio (COV-1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- tools/observer-coverage-checker.mjs | 57 +++++++++++++----------- tools/observer-coverage-checker.test.mjs | 30 ++++++++++--- 2 files changed, 54 insertions(+), 33 deletions(-) diff --git a/tools/observer-coverage-checker.mjs b/tools/observer-coverage-checker.mjs index 09c25c1b..6c7cb09c 100644 --- a/tools/observer-coverage-checker.mjs +++ b/tools/observer-coverage-checker.mjs @@ -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 }; } diff --git a/tools/observer-coverage-checker.test.mjs b/tools/observer-coverage-checker.test.mjs index 641e9634..b44d8ccf 100644 --- a/tools/observer-coverage-checker.test.mjs +++ b/tools/observer-coverage-checker.test.mjs @@ -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); }); });