384 lines
18 KiB
JavaScript
384 lines
18 KiB
JavaScript
// 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 <dir>
|
|
// → {"proposals":[…], "reviews_read": N, "skipped": [...]} on stdout, exit 0; exit 2 = could not run
|
|
// node lib/review/recurrence.mjs apply --repo <dir> --rule-key <KEY> [--line "<edited line>"]
|
|
// → {"written":true|false, "reason":…} on stdout; 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, dirname } from 'node:path';
|
|
import { execFileSync } 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 = '<!-- voyage:review-lessons:start -->';
|
|
export const MARK_END = '<!-- voyage:review-lessons:end -->';
|
|
export const POINTER_START = '<!-- voyage:review-lessons-pointer:start -->';
|
|
export const POINTER_END = '<!-- voyage:review-lessons-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;
|
|
|
|
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" -- <changed files>` 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/<dir>/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('<!--') || 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 ---------------------------------------------
|
|
|
|
function atomicWriteText(path, text) {
|
|
// Write THROUGH a symlink (CLAUDE.md -> AGENTS.md is common): rename onto the
|
|
// real target, so the link survives and the target gets the change.
|
|
if (existsSync(path) && lstatSync(path).isSymbolicLink()) path = realpathSync(path);
|
|
mkdirSync(dirname(path), { recursive: true });
|
|
const tmp = `${path}.tmp-${process.pid}`;
|
|
writeFileSync(tmp, text);
|
|
renameSync(tmp, path);
|
|
}
|
|
|
|
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]
|
|
* @returns {{ written: boolean, reason: string, path?: string, pointer?: 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);
|
|
atomicWriteText(path, next);
|
|
|
|
let pointer = 'no-claude-md';
|
|
const claudePath = join(repo, 'CLAUDE.md');
|
|
if (existsSync(claudePath)) {
|
|
const updated = withPointer(readFileSync(claudePath, 'utf-8'));
|
|
if (updated === null) pointer = 'present';
|
|
else { atomicWriteText(claudePath, updated); pointer = 'written'; }
|
|
}
|
|
return { written: true, reason: 'written', path: relative(repo, path), pointer };
|
|
}
|
|
|
|
// ---- 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 <dir> | apply --repo <dir> --rule-key <KEY> [--line "<text>"]\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);
|
|
}
|
|
}
|