From 3ad5d0baea9cc34e23a226bda96881d789b2eecd Mon Sep 17 00:00:00 2001 From: Codex Date: Thu, 10 Sep 2026 00:06:55 +0300 Subject: [PATCH] ci: merge-candidate compares patch-ids without the review documents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- scripts/merge-candidate.mjs | 5 ++- scripts/mutation-gate.mjs | 11 ++++++ test/merge-candidate.test.mjs | 64 ++++++++++++++++++++++++++++++++++- 3 files changed, 78 insertions(+), 2 deletions(-) diff --git a/scripts/merge-candidate.mjs b/scripts/merge-candidate.mjs index ce3c98a0..ee40e5f7 100755 --- a/scripts/merge-candidate.mjs +++ b/scripts/merge-candidate.mjs @@ -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'; }, diff --git a/scripts/mutation-gate.mjs b/scripts/mutation-gate.mjs index 21028c59..a1c146b1 100644 --- a/scripts/mutation-gate.mjs +++ b/scripts/mutation-gate.mjs @@ -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', diff --git a/test/merge-candidate.test.mjs b/test/merge-candidate.test.mjs index 942feb6a..008e2861 100755 --- a/test/merge-candidate.test.mjs +++ b/test/merge-candidate.test.mjs @@ -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 }); + } +});