ci: merge-candidate compares patch-ids without the review documents

The candidate is the branch tip, which already carries the round's
CODE-REVIEW-N-rK.md; the material the reviewer read does not. With
docs/reviews in the diff the two patch-ids never matched once dev had
moved, so every green candidate went back to review whenever another
task published its own document in the meantime — #514 looped twice on
09.09 and #508 only merged when dev happened to stand still. The
patch-id now excludes docs/reviews, exactly like `reviewedFresh` next to
it; a real change of the patch under rebase still returns the task.

Mutant: merge-rereviews-own-review-doc.

Issue: #516
User-Visible: no
This commit is contained in:
Codex
2026-09-09 21:39:17 +00:00
committed by claude[bot]
parent 440460255b
commit 3ad5d0baea
3 changed files with 78 additions and 2 deletions
+4 -1
View File
@@ -110,8 +110,11 @@ export function realOps({ repo, token, workflow = 'validate.yml', sleep = (ms) =
revParse: (ref) => must(git('rev-parse', ref), `rev-parse ${ref}`),
mergeBase: (a, b) => must(git('merge-base', a, b), 'merge-base'),
diffNames: (from, to, pathspec = []) => must(git('diff', '--name-only', from, to, '--', ...pathspec), 'diff').split('\n').filter(Boolean),
// Документы ревью — не часть патча (#516): кандидат несёт свой
// CODE-REVIEW-N-rK.md, материал — нет, и без pathspec их patch-id
// расходились на каждом сдвиге dev; `reviewedFresh` судит так же.
patchId: (from, to) => {
const diff = must(git('diff', '--full-index', from, to), 'diff');
const diff = must(git('diff', '--full-index', from, to, '--', '.', ':!docs/reviews'), 'diff');
const r = spawnSync('git', ['patch-id', '--stable'], { input: diff, encoding: 'utf8' });
return (r.stdout || '').trim().split(' ')[0] || 'empty';
},
+11
View File
@@ -8203,6 +8203,17 @@ const MUTANT_DEFINITIONS = [
replace: " if (false) { // mutant: cancelled counts as red",
}],
},
{
id: 'merge-rereviews-own-review-doc',
guard: 'node --test test/merge-candidate.test.mjs',
because: 'the candidate carries its own review document and the material does not; a patch-id '
+ 'that counts docs/reviews sends every green candidate back to review whenever dev moved (#516)',
patches: [{
file: 'scripts/merge-candidate.mjs',
find: " const diff = must(git('diff', '--full-index', from, to, '--', '.', ':!docs/reviews'), 'diff');",
replace: " const diff = must(git('diff', '--full-index', from, to), 'diff'); // mutant: review docs count",
}],
},
{
id: 'merge-trusts-cancelled-dispatch',
guard: 'node --test test/merge-candidate.test.mjs',
+63 -1
View File
@@ -1,7 +1,7 @@
import test from 'node:test';
import assert from 'node:assert/strict';
import { execFileSync, spawnSync } from 'node:child_process';
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
@@ -265,3 +265,65 @@ test('#510 r2 M1: realOps.waitValidate with only a cancelled dispatch reports mi
const r = await ops.waitValidate('c'.repeat(40), { event: 'workflow_dispatch' });
assert.equal(r.result, 'missing');
});
// ---------- #516: the candidate carries its own review document; dev moves by other documents ----------
test('#516 AC1: dev moved only by review documents and the branch carries its own — patch-id equal, merge goes through Validate, not re-review', async () => {
const dir = mkdtempSync(join(tmpdir(), 'hp-merge-516-'));
try {
const origin = join(dir, 'origin.git');
const work = join(dir, 'work');
execFileSync('git', ['init', '-q', '--bare', origin]);
execFileSync('git', ['clone', '-q', origin, work]);
const cfg = ['-c', 'user.name=t', '-c', 'user.email=t@x'];
const git = (cwd, ...args) => execFileSync('git', ['-C', cwd, ...cfg, ...args], { encoding: 'utf8' }).trim();
const commit = (msg) => execFileSync('git', ['-C', work, ...cfg, 'commit', '-q', '-am', msg]);
mkdirSync(join(work, 'docs', 'reviews'), { recursive: true });
writeFileSync(join(work, 'a.mjs'), 'export const a = 20;\n');
writeFileSync(join(work, 'docs', 'reviews', '.keep'), '');
git(work, 'add', '.');
commit('base');
git(work, 'branch', '-M', 'dev');
git(work, 'push', '-q', '-u', 'origin', 'dev');
// ветка задачи: код + (позже) её собственный документ ревью
git(work, 'checkout', '-q', '-b', 'issue/9-fix');
writeFileSync(join(work, 'a.mjs'), 'export const a = 21;\n');
commit('fix');
const material = git(work, 'rev-parse', 'HEAD');
writeFileSync(join(work, 'docs', 'reviews', 'CODE-REVIEW-9-r1.md'), '# CODE-REVIEW-9-r1\nVerdict: green\n');
git(work, 'add', '.');
commit('docs: review document for #9');
git(work, 'push', '-q', '-u', 'origin', 'issue/9-fix');
// dev двинулся чужим документом ревью — ровно то, что делает каждый паблиш конвейера
git(work, 'checkout', '-q', 'dev');
writeFileSync(join(work, 'docs', 'reviews', 'CODE-REVIEW-8-r2.md'), '# CODE-REVIEW-8-r2\n');
git(work, 'add', '.');
commit('docs: review document for #8');
git(work, 'push', '-q', 'origin', 'dev');
const calls = [];
const ops = realOps({ repo: 'x/y', token: 'none' });
ops.pushWithLease = (sha, ref, expected) => {
calls.push(['push', ref, expected]);
const r = spawnSync('git', ['-C', work, 'push', '-q', `--force-with-lease=refs/heads/${ref}:${expected}`, 'origin', `${sha}:refs/heads/${ref}`], { encoding: 'utf8' });
return r.status === 0;
};
ops.dispatchValidate = (ref) => { calls.push(['dispatch', ref]); };
ops.waitValidate = async (sha) => { calls.push(['validate', sha]); return { result: 'green', url: 'https://run/1' }; };
ops.comment = (issue, body) => { calls.push(['comment', body.slice(0, 60)]); };
ops.log = () => {};
const inWork = (fn) => (...args) => { const cwd = process.cwd(); process.chdir(work); try { return fn(...args); } finally { process.chdir(cwd); } };
for (const name of ['fetch', 'revParse', 'mergeBase', 'diffNames', 'patchId', 'rebaseOnto']) ops[name] = inWork(ops[name]);
const r = await mergeCandidate({ branch: 'issue/9-fix', material, issue: 9, ops });
assert.equal(r.action, 'push', JSON.stringify(calls));
assert.ok(calls.some((c) => c[0] === 'validate'), 'кандидат прошёл Validate');
assert.ok(!calls.some((c) => c[0] === 'comment' && /patch-id/.test(c[1])), 'нет возврата «дифф изменился»');
const devTip = git(work, 'rev-parse', 'origin/dev');
assert.match(git(work, 'show', `${devTip}:a.mjs`), /a = 21/);
assert.equal(git(work, 'cat-file', '-t', `${devTip}:docs/reviews/CODE-REVIEW-9-r1.md`), 'blob', 'документ раунда уехал вместе с кодом');
assert.equal(git(work, 'cat-file', '-t', `${devTip}:docs/reviews/CODE-REVIEW-8-r2.md`), 'blob');
} finally {
rmSync(dir, { recursive: true, force: true });
}
});