From b856dd33c88199a0f01911787c5c7555b2fb8c61 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 26 Sep 2026 09:52:56 +0300 Subject: [PATCH] infra(process): fast-forward merge rebuilds the review index too (#657 r1 H1) The task branch no longer carries docs/reviews/INDEX.md (1b), and merge-candidate rebuilt it only inside rebaseOnto. When dev did not move the fast-forward pushed the stale index and reviews_index would turn dev red. freshIndex(tip) commits the rebuilt index on top of the material before the push; the merge stays a fast-forward. Real-git test: fast-forward, then reviews-index --check on the dev head is green. Mutant merge-ff-skips-review-index. Issue: #657 User-Visible: no --- PROCESS.md | 5 +- scripts/merge-candidate.mjs | 16 ++++++- scripts/mutation-registry.mjs | 6 +++ test/merge-candidate.test.mjs | 86 +++++++++++++++++++++++++++++++++-- 4 files changed, 105 insertions(+), 8 deletions(-) diff --git a/PROCESS.md b/PROCESS.md index 6faf6492..be231357 100644 --- a/PROCESS.md +++ b/PROCESS.md @@ -364,8 +364,9 @@ S1-new → S2-analysis → S3-spec → S4-spec-review ⟲ → S5-ready → документ — issue, этап, раунд, вердикт, число High/Medium, заголовки находок, файлы из находок (искать по имени файла: `grep form-kit docs/reviews/INDEX.md`). Файл генерируется `node scripts/reviews-index.mjs` и пересобирается **только -коммитами, идущими в `dev`** (#657, решение 1б): слиянием кандидата после -ребейза (`--commit-if-stale`, коммит класса C) и публикацией документа ревью ТЗ +коммитами, идущими в `dev`** (#657, решение 1б): слиянием кандидата — +после ребейза, а если `dev` не двигался, то поверх материала перед +fast-forward (`--commit-if-stale`, коммит класса C) — и публикацией документа ревью ТЗ прямо в `dev`. В ветке задачи индекс не пересобирается — ни при приведении к dev, ни при публикации документа код-ревью: иначе две параллельные задачи конфликтуют на нём по построению. Руками не правится. Конфликт ребейза, в котором **все** пути — diff --git a/scripts/merge-candidate.mjs b/scripts/merge-candidate.mjs index 39128c6a..62a2b802 100755 --- a/scripts/merge-candidate.mjs +++ b/scripts/merge-candidate.mjs @@ -167,6 +167,17 @@ export function realOps({ must(exec(process.execPath, [REVIEWS_INDEX_SCRIPT, '--dir=docs/reviews', '--commit-if-stale', `--issue=${issue}`]), 'reviews-index --commit-if-stale'); return must(git('rev-parse', 'HEAD'), 'rev-parse HEAD'); }, + // #657 (1б) r1 H1: документ ревью ветки задачи больше не несёт индекс — + // его пересобирают только коммиты, идущие в dev. Ребейз делает это сам + // (выше); fast-forward, когда dev не двигался, ребейза не знает, и без + // этого шага в dev уехал бы устаревший INDEX.md — красный `reviews_index` + // на голове dev. Коммит индекса — doc-коммит конвейера поверх материала, + // слияние остаётся fast-forward. + freshIndex: (tip) => { + must(git('checkout', '-q', '-B', 'merge-into-dev', tip), 'checkout'); + must(exec(process.execPath, [REVIEWS_INDEX_SCRIPT, '--dir=docs/reviews', '--commit-if-stale', `--issue=${issue}`]), 'reviews-index --commit-if-stale'); + return must(git('rev-parse', 'HEAD'), 'rev-parse HEAD'); + }, pushWithLease: (sha, ref, expected) => { const r = git('push', '-q', `--force-with-lease=refs/heads/${ref}:${expected}`, pushUrl, `${sha}:refs/heads/${ref}`); if (r.status === 0) return true; @@ -249,10 +260,11 @@ export async function mergeCandidate({ branch, material, issue, ops, maxAttempts ops.log(`попытка ${attempt}: dev@${devNow.slice(0, 8)}, база материала ${materialBase.slice(0, 8)}, dev ${devMoved ? 'двигался' : 'на месте'}`); if (!devMoved) { - const pushed = ops.pushWithLease(tip, 'dev', devNow); + const target = ops.freshIndex(tip); + const pushed = ops.pushWithLease(target, 'dev', devNow); const decision = decideMerge({ fresh: true, devMoved: false, leaseRejected: !pushed }); if (decision.action === 'retry') continue; - return finish(decision, { candidate: tip, devNow }); + return finish(decision, { candidate: target, devNow }); } const candidate = ops.rebaseOnto(tip, 'origin/dev'); diff --git a/scripts/mutation-registry.mjs b/scripts/mutation-registry.mjs index 5118012f..0d058014 100644 --- a/scripts/mutation-registry.mjs +++ b/scripts/mutation-registry.mjs @@ -9497,6 +9497,12 @@ const MUTANT_DEFINITIONS = [ because: "#657: the freshness check has to run in the publishing orchestrator, not only in a unit", patches: [{ file: "scripts/release-prerelease.mjs", find: " assertCommittedBundleFresh(manifest, sourceFingerprint(root));\n", replace: "" }], }, + { + id: "merge-ff-skips-review-index", + guard: "node --test --test-name-pattern=\"#657 r1 H1\" test/merge-candidate.test.mjs", + because: "#657 r1 H1: the task branch no longer carries INDEX.md, so a fast-forward merge must rebuild it or dev turns red on reviews_index", + patches: [{ file: "scripts/merge-candidate.mjs", find: " const target = ops.freshIndex(tip);", replace: " const target = tip;" }], + }, { id: "provenance-skips-bundle-rule", guard: "node --test --test-name-pattern=\"#657 \u043f\u0440\u0430\u0432\u0438\u043b\u043e \u0438\u0441\u043f\u043e\u043b\u043d\u044f\u0435\u0442 validate-commit-provenance\" test/bundle-policy.test.mjs", diff --git a/test/merge-candidate.test.mjs b/test/merge-candidate.test.mjs index dec2e053..006e1f2e 100755 --- a/test/merge-candidate.test.mjs +++ b/test/merge-candidate.test.mjs @@ -4,6 +4,7 @@ import { execFileSync, spawnSync } from 'node:child_process'; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; import { MAX_ATTEMPTS, MAX_COMMAND_OUTPUT_BYTES, commentFor, decideMerge, mergeCandidate, realOps, sh, @@ -81,7 +82,7 @@ test('каждый исход, меняющий метку, объясняетс * dev по порядку (следующая после каждого отклонённого lease), ответы * Validate — по порядку кандидатов. */ -function fakeOps({ base = 'dev0', devTips = ['dev0'], validate = [], leaseRejects = 0, patchIds = {}, branchTip, material, conflictOnce = false }) { +function fakeOps({ base = 'dev0', devTips = ['dev0'], validate = [], leaseRejects = 0, patchIds = {}, branchTip, material, conflictOnce = false, indexStale = false }) { const calls = []; let devIndex = 0; let validateIndex = 0; @@ -105,6 +106,10 @@ function fakeOps({ base = 'dev0', devTips = ['dev0'], validate = [], leaseReject if (conflict) { conflict = false; return null; } return `cand-${tip}-on-${dev()}`; }, + freshIndex: (tip) => { + calls.push(['index', tip]); + return indexStale ? `idx-${tip}` : tip; + }, pushWithLease: (sha, ref, expected) => { calls.push(['push', sha, ref, expected]); if (ref === 'dev' && rejects > 0) { rejects -= 1; devIndex += 1; return false; } @@ -131,6 +136,17 @@ test('dev не двигался: push кандидата как есть, с lea assert.ok(!ops.calls.some((c) => c[0] === 'validate'), 'без движения dev Validate не ждётся'); }); +test('#657 r1 H1: dev не двигался — индекс пересобирается поверх материала, в dev уходит вершина с индексом', async () => { + const ops = fakeOps({ devTips: ['dev0'], branchTip: 'mat', material: 'mat', indexStale: true }); + const r = await mergeCandidate({ branch: 'issue/1-x', material: 'mat', issue: 1, ops }); + assert.equal(r.action, 'fast-forward'); + assert.equal(r.candidate, 'idx-mat'); + const indexAt = ops.calls.findIndex((c) => c[0] === 'index'); + const pushAt = ops.calls.findIndex((c) => c[0] === 'push'); + assert.ok(indexAt >= 0 && indexAt < pushAt, 'индекс пересобран до push'); + assert.deepEqual(ops.calls[pushAt], ['push', 'idx-mat', 'dev', 'dev0']); +}); + test('эксперимент аудита: dev двигался, ребейз чистый, patch-id равен — Validate ОБЯЗАТЕЛЕН до push', async () => { const ops = fakeOps({ devTips: ['dev1'], branchTip: 'mat', material: 'mat' }); const r = await mergeCandidate({ branch: 'issue/1-x', material: 'mat', issue: 1, ops }); @@ -239,7 +255,7 @@ test('на настоящем git: чистый ребейз с равным pat ops.comment = (issue, body) => { calls.push(['comment', body.slice(0, 40)]); }; 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]); + for (const name of ['fetch', 'revParse', 'mergeBase', 'diffNames', 'patchId', 'rebaseOnto', 'freshIndex']) ops[name] = inWork(ops[name]); const r = await mergeCandidate({ branch: 'issue/7-double', material, issue: 7, ops }); assert.equal(r.action, 'push', JSON.stringify(calls)); @@ -379,7 +395,7 @@ test('#516 AC1: dev moved only by review documents and the branch carries its ow 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]); + for (const name of ['fetch', 'revParse', 'mergeBase', 'diffNames', 'patchId', 'rebaseOnto', 'freshIndex']) 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)); @@ -453,7 +469,7 @@ test('#643 AC1: dev сдвинулся документами ревью дру 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]); + for (const name of ['fetch', 'revParse', 'mergeBase', 'diffNames', 'patchId', 'rebaseOnto', 'freshIndex']) 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)); @@ -496,3 +512,65 @@ test('#596: сбой запуска называет причину, а не п assert.match(r.stderr, /houseplan-no-such-command-596 не выполнился: ENOENT/); assert.equal(r.stdout, ''); }); + +// #657 r1 H1: документ код-ревью уезжает в ветку без индекса (1б), dev не +// двигался — fast-forward. Индекс на голове dev обязан быть свежим, иначе +// `reviews-index --check` в Validate красит dev на первом же тихом слиянии. +test('#657 r1 H1 на настоящем git: fast-forward несёт свежий INDEX.md, --check на голове dev зелёный', async () => { + const dir = mkdtempSync(join(tmpdir(), 'hp-merge-ff-index-')); + try { + const bare = join(dir, 'origin.git'); + execFileSync('git', ['init', '-q', '--bare', bare]); + const work = join(dir, 'work'); + execFileSync('git', ['clone', '-q', bare, 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', 'CODE-REVIEW-8-r1.md'), '# CODE-REVIEW-8-r1\n'); + writeFileSync(join(work, 'docs', 'reviews', 'INDEX.md'), buildIndex(join(work, 'docs', 'reviews'))); + 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'); + // публикация документа в ветку задачи — без индекса (1б) + 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'); + + const calls = []; + const ops = realOps({ repo: 'x/y', token: 'none', issue: 9 }); + 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', 'freshIndex']) ops[name] = inWork(ops[name]); + + const r = await mergeCandidate({ branch: 'issue/9-fix', material, issue: 9, ops }); + assert.equal(r.action, 'fast-forward', JSON.stringify(calls)); + git(work, 'fetch', '-q', 'origin'); + const devTip = git(work, 'rev-parse', 'origin/dev'); + assert.equal(devTip, r.candidate); + assert.equal(git(work, 'merge-base', '--is-ancestor', material, devTip) , '', 'материал — предок головы dev: слияние fast-forward'); + const index = git(work, 'show', `${devTip}:docs/reviews/INDEX.md`); + assert.match(index, /CODE-REVIEW-9-r1\.md/, 'документ раунда виден через индекс'); + assert.match(index, /CODE-REVIEW-8-r1\.md/); + git(work, 'checkout', '-q', '--detach', devTip); + const check = spawnSync(process.execPath, [fileURLToPath(new URL('../scripts/reviews-index.mjs', import.meta.url)), '--dir=docs/reviews', '--check'], { cwd: work, encoding: 'utf8' }); + assert.equal(check.status, 0, `reviews-index --check на голове dev: ${check.stdout}${check.stderr}`); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +});