// lib/review/recurrence.mjs // P5 — the project's memory, fed by review (ordre 5). // // A finding class (the same catalogue `rule_key`) that shows up in TWO review.md // files in the target repo, as two DIFFERENT findings (distinct finding IDs), // becomes ONE proposal; one sighting is no proposal, and one unfixed finding // seen again in the next iteration is a persisting defect, not a recurrence. // Catch-all keys (route null) and findings whose key was normalised from an // unknown one (`original_rule_key`) are never counted. // The history is the review.md files the repo already has — the current // `.claude/projects/*/review.md` plus, where git tracks them, their earlier // committed versions. No new store is created. // // Routing (the catalogue's `memory_route`): mechanical → the lesson names a // deterministic check; judgement → a line in the target repo's `.claude/rules/` // file; CLAUDE.md gets only a pointer. Review-hygiene classes (route null) are // never proposed. Format of every line: mistake · fix · how to check. // // Writes: only between Voyage's own markers, under a size budget; past the // budget the write is refused and the lesson belongs in a skill (operator's // call). Every line is approved by the operator first (/trekreview Phase 8.7, // AskUserQuestion) — this module never decides that on its own. // // Borrowed (credited, MIT where it applies): // - routing check / standard / pointer: Pocock `skills/in-progress/retro/SKILL.md:19,41` // - the three-part line "mistake · fix · how to check": ai-native-sdlc `assets/CLAUDE.md:27-29` // - the managed block between START/END markers: Spec Kit `extensions/agent-context/README.md:5` // - evidence (two finding IDs) per proposal, schema-check before writing: // Kiro https://kiro.dev/blog/trust-agent-triage/ (docs only, closed product) // - review → memory, and the size budget ("or make a skill"): Boris Cherny's // public talks/posts (sources A, B; see docs/praksisanalyse) // // CLI: // node lib/review/recurrence.mjs propose --repo // → {"proposals":[…], "reviews_read": N, "skipped": [...]} on stdout, exit 0; exit 2 = could not run // node lib/review/recurrence.mjs apply --repo --rule-key [--line ""] // → {"written":true|false, "reason":…, "pointer":…} on stdout (the pointer result, // see applyLesson); exit 0 written, 1 refused, 2 could not run import { readFileSync, writeFileSync, renameSync, existsSync, mkdirSync, readdirSync, statSync, realpathSync, lstatSync, } from 'node:fs'; import { join, relative, isAbsolute, sep, delimiter, resolve, dirname } from 'node:path'; import { execFileSync, spawnSync } from 'node:child_process'; import { validateReviewContent } from '../validators/review-validator.mjs'; import { extractFindingsBlock } from './findings-schema.mjs'; import { getRule } from './rule-catalogue.mjs'; import { isMain } from '../util/is-main.mjs'; export const LESSONS_DIR = join('.claude', 'rules'); export const LESSONS_FILE_NAME = 'voyage-review-lessons.md'; export const MARK_START = ''; export const MARK_END = ''; export const POINTER_START = ''; export const POINTER_END = ''; export const POINTER_LINE = `Recurring review lessons (operator-approved, managed by Voyage /trekreview): \`${LESSONS_DIR}/${LESSONS_FILE_NAME}\`.`; /** Size budget for the managed block. Past it, a lesson belongs in a skill. */ export const DEFAULT_BUDGET_ENTRIES = 20; export const DEFAULT_BUDGET_CHARS = 6000; export const LINE_MAX = 700; export const RECURRENCE_THRESHOLD = 2; export const BLOCK_HEADER = 'Each line: a finding class /trekreview saw in two reviews, approved by the operator. Format: mistake · fix · how to check.'; /** How to check a mechanical class. Judgement classes get the review itself. */ const CHECK_HINTS = Object.freeze({ MISSING_TEST: 'a coverage check on the changed files with the repo\'s own test tool — red when a changed source file has no test that exercises it.', PLACEHOLDER_IN_CODE: '`git grep -nE "TODO|FIXME|XXX|debugger" -- ` returns nothing.', UNDECLARED_DEPENDENCY: 'every import in the changed files resolves to a declared dependency (the repo\'s own dependency checker).', BROKEN_SUCCESS_CRITERION: 'run each Success Criterion\'s verification command before the step is called done; only exit 0 counts.', }); // ---- reading the history ---------------------------------------------------- /** * Pull the kept findings out of one review.md text. * @param {string} text * @returns {{ ok: boolean, identity?: string, findings?: object[], reason?: string }} */ export function parseReview(text) { const r = validateReviewContent(text, { strict: false }); if (!r.valid) return { ok: false, reason: (r.errors ?? []).map((e) => e.code).join(',') || 'invalid' }; const fm = r.parsed.frontmatter ?? {}; const block = extractFindingsBlock(r.parsed.body ?? ''); if (block === null) return { ok: false, reason: 'no-json-block' }; let payload; try { payload = JSON.parse(block); } catch { return { ok: false, reason: 'json-parse-error' }; } const findings = (Array.isArray(payload?.findings) ? payload.findings : []) .map((f) => ({ ...f, rule_key: f?.rule_key ?? f?.rule })) .filter((f) => typeof f.rule_key === 'string'); const identity = `${fm.project_dir ?? fm.slug ?? '?'}@${fm.scope_sha_end ?? '?'}`; return { ok: true, identity, findings }; } function git(repo, args) { return execFileSync('git', ['-C', repo, ...args], { encoding: 'utf-8', stdio: ['ignore', 'pipe', 'ignore'] }); } /** * Every review.md the repo already holds: current files under * `.claude/projects//review.md`, plus earlier committed versions of those * files when git tracks them. Deduplicated by review identity * (project_dir @ scope_sha_end). * @param {string} repo * @returns {{ reviews: Array<{ identity: string, source: string, findings: object[] }>, skipped: Array<{ source: string, reason: string }> }} */ export function collectReviews(repo) { const sources = []; const projects = join(repo, '.claude', 'projects'); if (existsSync(projects)) { for (const name of readdirSync(projects).sort()) { const p = join(projects, name, 'review.md'); if (existsSync(p) && statSync(p).isFile()) { sources.push({ source: relative(repo, p), text: readFileSync(p, 'utf-8') }); } } } // Earlier versions (review.md is overwritten per iteration; git keeps the rest). for (const { source } of [...sources]) { let shas = []; try { shas = git(repo, ['log', '--format=%H', '--', source]).split('\n').filter(Boolean); } catch { shas = []; } for (const sha of shas) { try { // `./` makes the path relative to --repo, not to the enclosing git root. sources.push({ source: `${source}@${sha.slice(0, 12)}`, text: git(repo, ['show', `${sha}:./${source}`]) }); } catch { /* not in that commit's tree */ } } } const seen = new Set(); const reviews = []; const skipped = []; for (const s of sources) { const parsed = parseReview(s.text); if (!parsed.ok) { skipped.push({ source: s.source, reason: parsed.reason }); continue; } if (seen.has(parsed.identity)) continue; seen.add(parsed.identity); reviews.push({ identity: parsed.identity, source: s.source, findings: parsed.findings }); } return { reviews, skipped }; } // ---- the managed block ------------------------------------------------------ function lessonsPath(repo) { return join(repo, LESSONS_DIR, LESSONS_FILE_NAME); } function count(text, needle) { return text.split(needle).length - 1; } /** The managed block, null when absent, or { damaged: true } when the markers are not exactly one START before one END. */ function readBlock(text) { const starts = count(text, MARK_START); const ends = count(text, MARK_END); if (starts === 0 && ends === 0) return null; const start = text.indexOf(MARK_START); const end = text.indexOf(MARK_END); if (starts !== 1 || ends !== 1 || end < start) return { damaged: true }; return { start, end, inner: text.slice(start + MARK_START.length, end) }; } function blockEntries(inner) { return inner.split('\n').filter((l) => l.startsWith('- [')); } /** The rule_keys already written as lessons. */ export function writtenClasses(repo) { const p = lessonsPath(repo); if (!existsSync(p)) return new Set(); const block = readBlock(readFileSync(p, 'utf-8')); if (!block || block.damaged) return new Set(); const keys = new Set(); for (const line of blockEntries(block.inner)) { const m = line.match(/^- \[([A-Z_]+)\]/); if (m) keys.add(m[1]); } return keys; } // ---- proposals -------------------------------------------------------------- function clip(s, n) { const t = String(s ?? '').replace(//g, '').replace(/[\u0000-\u001f\u007f\u2028\u2029\s]+/g, ' ').trim(); return t.length > n ? `${t.slice(0, n - 1)}…` : t; } function composeLine(rule, evidence) { const seen = evidence.map((e) => `"${clip(e.title ?? e.rule_key, 80)}" (${clip(e.file, 60)})`).join(', '); const fix = clip(evidence[evidence.length - 1].recommended_action ?? evidence.find((e) => e.recommended_action)?.recommended_action ?? 'Address the class before the step is called done.', 160); const how = rule.memory_route === 'check' ? CHECK_HINTS[rule.rule_key] ?? `a deterministic check for ${rule.rule_key}.` : `/trekreview flags it as ${rule.rule_key}; read the diff for it before the step is called done.`; const ids = evidence.map((e) => clip(String(e.id ?? '').slice(0, 8), 8)).join(', '); const body = `Mistake: ${clip(rule.description, 160)} Seen: ${seen}. · Fix: ${fix} · How to check: ${how} (evidence: ${ids})`; const line = `- [${rule.rule_key}] (${rule.memory_route}) ${body}`; if (line.length <= LINE_MAX) return line; // Shorten the free-text parts, never the three labels. const short = evidence.map((e) => `"${clip(e.title ?? e.rule_key, 40)}" (${clip(e.file, 40)})`).join(', '); return `- [${rule.rule_key}] (${rule.memory_route}) Mistake: ${clip(rule.description, 120)} Seen: ${short}. · Fix: ${clip(fix, 120)} · How to check: ${clip(how, 160)} (evidence: ${ids})`; } /** * Validate a lesson line (generated or operator-edited). Schema-check before * any write (Kiro's lesson-capture fix). * @param {string} line * @returns {{ valid: boolean, reason?: string }} */ export function validateLessonLine(line) { if (typeof line !== 'string' || line.trim().length === 0) return { valid: false, reason: 'empty' }; if (/[\u0000-\u001f\u007f\u2028\u2029]/.test(line)) return { valid: false, reason: 'control or line-break character' }; if (line.length > LINE_MAX) return { valid: false, reason: `longer than ${LINE_MAX}` }; if (line.includes('')) return { valid: false, reason: 'marker text' }; const a = line.indexOf('Mistake:'); const b = line.indexOf('Fix:'); const c = line.indexOf('How to check:'); if (a === -1 || b === -1 || c === -1 || !(a < b && b < c)) { return { valid: false, reason: 'needs "Mistake:", "Fix:", "How to check:" in that order' }; } return { valid: true }; } /** * Find recurring finding classes in the repo's review history. * @param {string} repo * @returns {{ proposals: object[], reviews_read: number, skipped: object[] }} */ export function proposeLessons(repo) { const { reviews, skipped } = collectReviews(repo); const already = writtenClasses(repo); const byKey = new Map(); for (const review of reviews) { const perKey = new Map(); for (const f of review.findings) { if (f.original_rule_key !== undefined) continue; // normalised from an unknown key: not a class if (!perKey.has(f.rule_key)) perKey.set(f.rule_key, []); perKey.get(f.rule_key).push(f); } for (const [key, fs] of perKey) { if (!byKey.has(key)) byKey.set(key, []); byKey.get(key).push({ review: review.identity, source: review.source, findings: fs }); } } const proposals = []; for (const [key, sightings] of [...byKey].sort(([a], [b]) => a.localeCompare(b))) { // Two reviews, and two DIFFERENT findings: pick, per review, a finding whose // id has not been used yet. The same unfixed finding twice is one defect. const usedIds = new Set(); const evidence = []; for (const s of sightings) { const f = s.findings.find((x) => !usedIds.has(x.id ?? `${x.file}:${x.line}`)); if (!f) continue; usedIds.add(f.id ?? `${f.file}:${f.line}`); evidence.push({ ...f, review: s.review, source: s.source }); } if (evidence.length < RECURRENCE_THRESHOLD) continue; const rule = getRule(key); if (!rule || rule.memory_route === null || rule.memory_route === undefined) continue; if (already.has(key)) continue; const ev = evidence.slice(0, RECURRENCE_THRESHOLD); const line = composeLine(rule, ev); if (!validateLessonLine(line).valid) continue; // never propose a line apply would refuse proposals.push({ rule_key: key, route: rule.memory_route, target: `${LESSONS_DIR}/${LESSONS_FILE_NAME}`, line, evidence: evidence.map((e) => ({ id: e.id ?? null, review: e.review, source: e.source, file: e.file ?? null, title: e.title ?? null })), }); } return { proposals, reviews_read: reviews.length, skipped }; } // ---- writing an approved lesson --------------------------------------------- /** lstat that answers null for a path that does not exist. */ function lstatOrNull(path) { try { return lstatSync(path); } catch (e) { if (e && e.code === 'ENOENT') return null; throw e; } } /** * Write `text` to `/` atomically, never outside the repo (R3). * Each directory of `rel` is checked one level at a time, parent first: an * existing one must resolve (realpath) to the repo's real path or inside it; a * missing one is created alone (non-recursive) once its parent passed, so no * directory is ever made through a link that leaves the repo. A file that is * itself a symlink is refused, never written through. * @param {string} repo * @param {string} rel path relative to the repo * @param {string} text * @returns {{ written: true } | { written: false, reason: string }} */ function atomicWriteText(repo, rel, text) { const repoReal = realpathSync(repo); const inside = (real) => { const r = relative(repoReal, real); return r === '' || (r !== '..' && !r.startsWith(`..${sep}`) && !isAbsolute(r)); }; const parts = rel.split(/[\\/]+/).filter(Boolean); let dir = repo; for (let i = 0; i < parts.length - 1; i += 1) { dir = join(dir, parts[i]); const shown = parts.slice(0, i + 1).join('/'); if (lstatOrNull(dir)) { const real = realpathSync(dir); if (!inside(real)) { return { written: false, reason: `directory-outside-repo: ${shown} points outside the repo (${real}); nothing written` }; } } else { mkdirSync(dir); } } const path = join(repo, ...parts); const st = lstatOrNull(path); if (st && st.isSymbolicLink()) { return { written: false, reason: `symlink-refused: ${parts.join('/')} is a symlink; it is not written through` }; } const tmp = `${path}.tmp-${process.pid}`; writeFileSync(tmp, text); renameSync(tmp, path); return { written: true }; } /** * Is the lessons file ignored by git? `git check-ignore -q` exit 0 = ignored, * 1 = not ignored; a spawn error or any exit other than 0, 1 or 128 is a * failed check. Exit 128 means "not a git repository" only some of the time: * inside a repository it also comes from a path beyond a symbolic link, a * broken `.git/HEAD` or a `GIT_DIR` that does not exist (CKI), and the probe's * own `git rev-parse --git-dir` exits 128 too on dubious ownership, a bad * `.git/config` or a garbage `.git/HEAD`. So on 128 a * probe decides (see notARepoReason), and only a directory that is truly no * repository gives `no-git-repo`; everything else, including a probe that * throws, is `failed`. No branch reads git's message text, so the answer does * not depend on the language git speaks. Every `failed` note after a 128 * starts `git check-ignore exit 128: ` and adds the reason. * @returns {{ state: 'ignored' | 'not-ignored' | 'no-git-repo' | 'failed', note?: string }} */ function lessonsIgnored(repo, rel) { const r = spawnSync('git', ['-C', repo, 'check-ignore', '-q', '--', rel], { encoding: 'utf-8', stdio: ['ignore', 'ignore', 'pipe'] }); if (r.error) return { state: 'failed', note: `git check-ignore could not start: ${r.error.message}` }; if (r.status === 0) return { state: 'ignored' }; if (r.status === 1) return { state: 'not-ignored' }; const err = String(r.stderr ?? '').trim().split('\n')[0] ?? ''; if (r.status === 128) { const head = `git check-ignore exit 128: ${err}`; try { const why = notARepoReason(repo); return why === null ? { state: 'no-git-repo' } : { state: 'failed', note: `${head}; ${why}` }; } catch (e) { return { state: 'failed', note: `${head}; the repository check threw: ${e.message}` }; } } const how = r.status === null ? `signal ${r.signal}` : `exit ${r.status}`; return { state: 'failed', note: `git check-ignore ${how}${err ? `: ${err}` : ''}` }; } /** * After a check-ignore exit 128: null when `repo` is no git repository, else * the reason it may be one. `git rev-parse --git-dir` exit 0 proves a * repository; its exit 128 alone proves nothing (see lessonsIgnored), so then * the answer is null only when GIT_DIR is unset and no `.git` entry (file or * directory) is found by gitEntryDir. A spawn error or any other exit is a * reason too. May throw (realpath of a vanished repo); the caller reads that * as a failed check. * @returns {string | null} */ function notARepoReason(repo) { const p = spawnSync('git', ['-C', repo, 'rev-parse', '--git-dir'], { encoding: 'utf-8', stdio: ['ignore', 'ignore', 'ignore'] }); if (p.error) return `git rev-parse --git-dir could not start: ${p.error.message}`; if (p.status !== 128) return `git rev-parse --git-dir ${p.status === null ? `signal ${p.signal}` : `exit ${p.status}`}`; if (process.env.GIT_DIR !== undefined) return 'GIT_DIR is set'; const found = gitEntryDir(repo); return found === null ? null : `a .git entry exists in ${found}`; } /** * The directory holding a `.git` entry, walking up from `realpath(repo)`, or * null. `realpath(repo)` itself is always checked; the walk stops before the * nearest proper ancestor that equals a GIT_CEILING_DIRECTORIES entry (split * on the path delimiter, empty entries skipped, each entry realpath'd, or * resolved when that throws), and without one it goes up to and including * `/`. An entry equal to `realpath(repo)` stops nothing, as in git. Unlike * git, the walk does not stop at a filesystem boundary: crossing one can only * find a `.git` git would not, which reads as a failed check (the safe side). * @returns {string | null} */ function gitEntryDir(repo) { const ceilings = new Set(String(process.env.GIT_CEILING_DIRECTORIES ?? '') .split(delimiter) .filter((e) => e !== '') .map((e) => { try { return realpathSync(e); } catch { return resolve(e); } })); let dir = realpathSync(repo); for (;;) { if (existsSync(join(dir, '.git'))) return dir; const up = dirname(dir); if (up === dir || ceilings.has(up)) return null; dir = up; } } export function withPointer(claudeText) { if (claudeText.includes(POINTER_START) || claudeText.includes(POINTER_END)) return null; const sep = claudeText.endsWith('\n') ? '\n' : '\n\n'; return `${claudeText}${sep}${POINTER_START}\n${POINTER_LINE}\n${POINTER_END}\n`; } /** * Write ONE operator-approved lesson. Call only after the operator said yes to * this exact line. * @param {string} repo * @param {string} ruleKey * @param {{ line?: string, budgetEntries?: number, budgetChars?: number }} [opts] * The result's `pointer` (when written): 'written' | 'present' | 'no-claude-md' | * 'symlink-skipped' (CLAUDE.md is a symlink, DJ4) | 'ignored-skipped' (git * ignores the lessons file, PTR) | 'check-failed' (git check-ignore did not * answer, or answered exit 128 for a directory that may be a repository, see * lessonsIgnored; `pointer_note` says how, and always names check-ignore). * Only a directory that is truly no repository gets the pointer without the * check. * Nothing is written through a symlinked * lessons file or a lessons directory outside the repo (R3): written false. * @returns {{ written: boolean, reason: string, path?: string, pointer?: string, pointer_note?: string }} */ export function applyLesson(repo, ruleKey, opts = {}) { const budgetEntries = opts.budgetEntries ?? DEFAULT_BUDGET_ENTRIES; const budgetChars = opts.budgetChars ?? DEFAULT_BUDGET_CHARS; if (writtenClasses(repo).has(ruleKey)) return { written: false, reason: 'already-present' }; const proposal = proposeLessons(repo).proposals.find((p) => p.rule_key === ruleKey); if (!proposal) return { written: false, reason: 'no-recurrence' }; let line = proposal.line; if (opts.line !== undefined) { const body = String(opts.line).replace(/^- \[[A-Z_]+\]\s*(\((check|rule)\)\s*)?/, ''); line = `- [${ruleKey}] (${proposal.route}) ${body.trim()}`; } const check = validateLessonLine(line); if (!check.valid) return { written: false, reason: 'invalid-line', detail: check.reason }; const path = lessonsPath(repo); const current = existsSync(path) ? readFileSync(path, 'utf-8') : ''; let block = readBlock(current); if (block && block.damaged) { return { written: false, reason: 'markers-damaged', detail: `${LESSONS_DIR}/${LESSONS_FILE_NAME} must hold exactly one start marker before one end marker — fix it by hand` }; } let base = current; if (!block) { const sep = current.length === 0 || current.endsWith('\n') ? '' : '\n'; base = `${current}${sep}${MARK_START}\n${BLOCK_HEADER}\n${MARK_END}\n`; block = readBlock(base); } const entries = blockEntries(block.inner); const innerAfter = `${block.inner.replace(/\n*$/, '')}\n${line}\n`; if (entries.length + 1 > budgetEntries || innerAfter.length > budgetChars) { return { written: false, reason: 'over-budget', detail: 'the managed block is full — the lessons belong in a skill now (operator decision)' }; } const next = base.slice(0, block.start + MARK_START.length) + innerAfter + base.slice(block.end); const rel = `${LESSONS_DIR}/${LESSONS_FILE_NAME}`.split(/[\\/]+/).join('/'); // git and the report use '/' const wrote = atomicWriteText(repo, rel, next); if (!wrote.written) return { written: false, reason: wrote.reason }; // The CLAUDE.md pointer. A symlinked CLAUDE.md is skipped, never written // through: committing the link's target would change the R6 exception (DJ4). // A lessons file git ignores gets no pointer: it would point at a file the // commit does not carry (PTR). A failed ignore check writes no pointer. let pointer = 'no-claude-md'; let pointerNote; const claudePath = join(repo, 'CLAUDE.md'); const claudeStat = lstatOrNull(claudePath); if (claudeStat && claudeStat.isSymbolicLink()) { pointer = 'symlink-skipped'; } else if (claudeStat) { const ignored = lessonsIgnored(repo, rel); if (ignored.state === 'ignored') pointer = 'ignored-skipped'; else if (ignored.state === 'failed') { pointer = 'check-failed'; pointerNote = ignored.note; } else { const updated = withPointer(readFileSync(claudePath, 'utf-8')); if (updated === null) pointer = 'present'; else { // Refused only when CLAUDE.md turned into a symlink since the lstat above. pointer = atomicWriteText(repo, 'CLAUDE.md', updated).written ? 'written' : 'symlink-skipped'; } } } const result = { written: true, reason: 'written', path: relative(repo, path), pointer }; if (pointerNote !== undefined) result.pointer_note = pointerNote; return result; } // ---- CLI -------------------------------------------------------------------- function flag(args, name) { const i = args.indexOf(name); return i === -1 ? undefined : args[i + 1]; } if (isMain(import.meta.url)) { const [cmd, ...args] = process.argv.slice(2); const repo = flag(args, '--repo'); const out = (o) => process.stdout.write(JSON.stringify(o, null, 2) + '\n'); if (!['propose', 'apply'].includes(cmd) || !repo) { process.stderr.write('Usage: recurrence.mjs propose --repo | apply --repo --rule-key [--line ""]\n'); process.exit(2); } if (!existsSync(repo) || !statSync(repo).isDirectory()) { process.stderr.write(`recurrence: repo not found: ${repo}\n`); process.exit(2); } try { if (cmd === 'propose') { out(proposeLessons(repo)); process.exit(0); } const key = flag(args, '--rule-key'); if (!key) { process.stderr.write('recurrence apply: --rule-key is required\n'); process.exit(2); } const r = applyLesson(repo, key, { line: flag(args, '--line') }); out(r); process.exit(r.written ? 0 : 1); } catch (e) { process.stderr.write(`recurrence: could not run: ${e.message}\n`); process.exit(2); } }