fix(review): an append of review answers to the approval log keeps the review current
This commit is contained in:
parent
5709957e10
commit
ce64851a0d
3 changed files with 424 additions and 6 deletions
|
|
@ -9,13 +9,22 @@
|
|||
// - the reviewed commit is HEAD, or an ancestor of HEAD;
|
||||
// - every commit between it and HEAD (side branches included, each diffed
|
||||
// against its first parent, `--no-relative --ignore-submodules=none`) changes only
|
||||
// `{prefix}.claude/rules/voyage-review-lessons.md` and `{prefix}CLAUDE.md`,
|
||||
// where {prefix} is the project's place in the repo (`git rev-parse
|
||||
// --show-prefix`);
|
||||
// `{prefix}.claude/rules/voyage-review-lessons.md`, `{prefix}CLAUDE.md`
|
||||
// and the approval log `.claude/voyage/approvals.jsonl` (LOG_RELPATH, at
|
||||
// the git top level, never under {prefix}), where {prefix} is the
|
||||
// project's place in the repo (`git rev-parse --show-prefix`);
|
||||
// - in each such commit the lessons file is unchanged outside its managed
|
||||
// block, every line its block gains is a lesson line (`validateLessonLine`)
|
||||
// or exactly `BLOCK_HEADER` (m4), and CLAUDE.md is unchanged or gains exactly the pointer block
|
||||
// (`withPointer` in lib/review/recurrence.mjs);
|
||||
// - in each such commit the approval log only gains appended review answers
|
||||
// (R6, fix round 4.5b2 part B): neither side is a symlink, the log after
|
||||
// is mode 100644 (also when the commit creates it), the log before
|
||||
// (empty when absent) is a byte prefix of the log after, and every added
|
||||
// line is a K2 record with exactly the record fields and kind `review`,
|
||||
// whose `hash` is its `recordHash`, whose `seq` follows the last record
|
||||
// before it and whose `prev` links to it (to GENESIS on an empty log);
|
||||
// any other change to the log is stale;
|
||||
// - a commit with no parent in that range fails closed, and so does a
|
||||
// symlinked CLAUDE.md (the pointer write goes through the link, so its
|
||||
// target shows as a changed path).
|
||||
|
|
@ -36,6 +45,7 @@
|
|||
import { spawnSync } from 'node:child_process';
|
||||
import { existsSync } from 'node:fs';
|
||||
import { LESSONS_FILE_NAME, MARK_START, MARK_END, BLOCK_HEADER, withPointer, validateLessonLine } from './recurrence.mjs';
|
||||
import { GENESIS, LOG_RELPATH, recordHash } from '../util/approval-log.mjs';
|
||||
|
||||
/**
|
||||
* A full commit id: 40 hex (SHA-1) or 64 hex (SHA-256), nothing shorter (M11).
|
||||
|
|
@ -46,6 +56,10 @@ export const FULL_SHA_RE = /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/i;
|
|||
const LESSONS_REL = `.claude/rules/${LESSONS_FILE_NAME}`; // posix, as `git diff` prints it
|
||||
const CLAUDE_REL = 'CLAUDE.md';
|
||||
const SYMLINK_MODE = '120000';
|
||||
const LOG_MODE = '100644'; // the approval log after a commit: a regular, non-executable file
|
||||
// The K2 record's keys (`FIELDS` plus `hash` in lib/util/approval-log.mjs, not exported there).
|
||||
const RECORD_KEYS = ['expires', 'hash', 'kind', 'prev', 'seq', 'sha256', 'subject', 'ts', 'v'].join(',');
|
||||
const HASH_RE = /^[0-9a-f]{64}$/;
|
||||
|
||||
/**
|
||||
* The review's `scope_sha_end` as written. Read from the frontmatter text, not
|
||||
|
|
@ -144,6 +158,61 @@ function blobAt(repo, rev, path) {
|
|||
return { ok: true, mode, text: blob.stdout };
|
||||
}
|
||||
|
||||
/**
|
||||
* One file's bytes in one tree (`blob` is its blobAt result): empty when absent.
|
||||
* Bytes, not text, so the prefix check below is a byte prefix.
|
||||
*/
|
||||
function bytesAt(repo, rev, path, blob) {
|
||||
if (blob.mode === null) return { ok: true, bytes: Buffer.alloc(0) };
|
||||
const r = spawnSync('git', ['cat-file', 'blob', `${rev}:${path}`], { cwd: repo, maxBuffer: 64 * 1024 * 1024 });
|
||||
if (r.status !== 0) return { ok: false, fail: gitFailed(`cat-file ${rev}:${path}`, r) };
|
||||
return { ok: true, bytes: r.stdout };
|
||||
}
|
||||
|
||||
/**
|
||||
* Does `after` hold `before` plus appended `review` records, each a valid link
|
||||
* of the K2 chain? `before`/`after` are Buffers. Returns true or false.
|
||||
*/
|
||||
function onlyAppendedReviewRecords(before, after) {
|
||||
if (after.length <= before.length || !after.subarray(0, before.length).equals(before)) return false;
|
||||
let added;
|
||||
try {
|
||||
added = new TextDecoder('utf-8', { fatal: true, ignoreBOM: true }).decode(after.subarray(before.length));
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
const beforeText = before.toString('utf8');
|
||||
// appendApproval adds a newline first when the log does not end with one.
|
||||
if (beforeText !== '' && !beforeText.endsWith('\n')) {
|
||||
if (!added.startsWith('\n')) return false;
|
||||
added = added.slice(1);
|
||||
}
|
||||
if (added === '' || !added.endsWith('\n')) return false;
|
||||
|
||||
let seq = 0;
|
||||
let prev = GENESIS;
|
||||
const beforeLines = beforeText.split('\n');
|
||||
if (beforeLines[beforeLines.length - 1] === '') beforeLines.pop();
|
||||
if (beforeLines.length > 0) {
|
||||
let last;
|
||||
try { last = JSON.parse(beforeLines[beforeLines.length - 1]); } catch { return false; }
|
||||
if (!last || typeof last !== 'object' || !Number.isInteger(last.seq) || !HASH_RE.test(String(last.hash))) return false;
|
||||
seq = last.seq;
|
||||
prev = last.hash;
|
||||
}
|
||||
for (const line of added.slice(0, -1).split('\n')) {
|
||||
let rec;
|
||||
try { rec = JSON.parse(line); } catch { return false; }
|
||||
if (!rec || typeof rec !== 'object' || Array.isArray(rec)) return false;
|
||||
if (Object.keys(rec).sort().join(',') !== RECORD_KEYS) return false;
|
||||
if (rec.kind !== 'review' || rec.seq !== seq + 1 || rec.prev !== prev) return false;
|
||||
if (recordHash(rec) !== rec.hash) return false;
|
||||
seq = rec.seq;
|
||||
prev = rec.hash;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Is the review of `sha` a review of what HEAD holds in `repo`?
|
||||
* @param {{ repo: string, sha: string }} args
|
||||
|
|
@ -204,7 +273,8 @@ export function checkReviewedCommit({ repo, sha } = {}) {
|
|||
const names = git(repo, ['diff', '--no-relative', '--ignore-submodules=none', '--name-only', '-z', '--no-renames', `${c}^1`, c]);
|
||||
if (names.status !== 0) return gitFailed(`diff ${short(c)}`, names);
|
||||
const paths = names.stdout.split('\0').filter(Boolean);
|
||||
const other = paths.find((p) => p !== lessons && p !== claude);
|
||||
// The approval log is the top-level one: `--no-relative` paths are from the top level.
|
||||
const other = paths.find((p) => p !== lessons && p !== claude && p !== LOG_RELPATH);
|
||||
if (other !== undefined) return stale(`${other} changed after the review (commit ${short(c)})`);
|
||||
|
||||
for (const path of paths) {
|
||||
|
|
@ -215,7 +285,17 @@ export function checkReviewedCommit({ repo, sha } = {}) {
|
|||
if (before.mode === SYMLINK_MODE || after.mode === SYMLINK_MODE) {
|
||||
return stale(`${path} is a symlink (commit ${short(c)}) — re-run /trekreview on this commit`);
|
||||
}
|
||||
if (path === lessons) {
|
||||
if (path === LOG_RELPATH) {
|
||||
// 7. The approval log: only appended review answers (R6).
|
||||
const was = bytesAt(repo, `${c}^1`, path, before);
|
||||
if (!was.ok) return was.fail;
|
||||
const now = bytesAt(repo, c, path, after);
|
||||
if (!now.ok) return now.fail;
|
||||
// A regular, non-executable file after the commit, on creation too.
|
||||
if (after.mode !== LOG_MODE || !onlyAppendedReviewRecords(was.bytes, now.bytes)) {
|
||||
return stale(`${path} changed other than by appended review answers (commit ${c.slice(0, 8)})`);
|
||||
}
|
||||
} else if (path === lessons) {
|
||||
// 5. Only the managed block may change.
|
||||
if (outsideBlock(before.text).trimEnd() !== outsideBlock(after.text).trimEnd()) {
|
||||
return stale(`${path} changed outside the managed block (commit ${short(c)})`);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue