diff --git a/.github/workflows/process.yml b/.github/workflows/process.yml index deb7e897..c317b76e 100644 --- a/.github/workflows/process.yml +++ b/.github/workflows/process.yml @@ -226,6 +226,20 @@ jobs: git checkout -q "origin/$branch" echo "материал ревью: ветка $branch, $(git rev-parse --short HEAD)" echo "name=$branch" >> "$GITHUB_OUTPUT" + # Якоря материала, устойчивые к ребейзу (#413, #414). SHA коммита + # ребейз меняет — содержимое нет: git адресует деревья и блобы их + # хешем. Снимаются здесь, где рабочая копия ЕЩЁ равна тому, что + # ревьюер прочтёт; в шаге публикации дерево уже сброшено на целевую + # ветку, и спрашивать его поздно. + echo "sha=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT" + echo "tree=$(git rev-parse 'HEAD^{tree}')" >> "$GITHUB_OUTPUT" + # ТЗ задачи: блоб переживает и ребейз, и удаление ветки, пока текст + # где-нибудь достижим. Файлов может не быть (инфраструктурная + # задача) или быть несколько (разбитое ТЗ) — тогда список пуст либо + # длиннее одного. + specs=$(git ls-files -s -- "docs/specs/${NUM}-*.md" \ + | awk '{print $2" "$4}' | tr '\n' ';') + echo "specs=$specs" >> "$GITHUB_OUTPUT" else echo "::warning::ветка issue/${NUM}-* не найдена на origin — ревью пойдёт по dev" echo "МАТЕРИАЛ НЕ ЗАПУШЕН" >> "$GITHUB_STEP_SUMMARY" @@ -629,6 +643,9 @@ jobs: STAGE: ${{ needs.guard.outputs.stage }} CYCLE: ${{ needs.guard.outputs.cycle }} SOURCE: ${{ runner.temp }}/review-document.md + MATERIAL_SHA: ${{ steps.branch.outputs.sha }} + MATERIAL_TREE: ${{ steps.branch.outputs.tree }} + MATERIAL_SPECS: ${{ steps.branch.outputs.specs }} run: | # Ветки задачи может не быть: у задач, размеченных до появления # конвейера, ТЗ лежит прямо в dev. Раньше шаг в этом случае молча @@ -677,6 +694,14 @@ jobs: mkdir -p docs/reviews cp "$SOURCE" "$doc" echo "документ взят из $SOURCE ($(wc -c < "$doc") байт)" + # Якоря дописывает конвейер, а не ревьюер (#414). Дисциплина здесь + # уже подводила: на #403 SHA сняли до ребейза и не сверили перед + # выводом — через раунд команда из §2.10 не работала. Машина же + # снимает якоря в момент чтения материала и ошибиться в них не + # может; блок помечен как машинный, чтобы никто не правил его руками. + node scripts/review-doc-guard.mjs --anchor="$doc" \ + --sha="$MATERIAL_SHA" --tree="$MATERIAL_TREE" \ + --branch="${BRANCH:-dev}" --specs="$MATERIAL_SPECS" else echo "::warning::$SOURCE не найден — документа для публикации нет" fi diff --git a/scripts/review-doc-guard.mjs b/scripts/review-doc-guard.mjs index 32896a9c..c650c827 100644 --- a/scripts/review-doc-guard.mjs +++ b/scripts/review-doc-guard.mjs @@ -24,7 +24,7 @@ * нечего, значит что-то пошло не так раньше. */ import { spawnSync } from 'node:child_process'; -import { readFileSync } from 'node:fs'; +import { readFileSync, writeFileSync } from 'node:fs'; export const REVIEW_DOC_ALLOWLIST = ['docs/reviews/']; @@ -105,6 +105,20 @@ export function citedMaterialShas(text, headerLines = REVIEW_HEADER_LINES) { return found; } +/** + * Якоря из машинного блока: то, чем раунд воспроизводится после ребейза. + * + * Разбор нарочно грубый — ищутся сорокасимвольные хеши в блоке, а не структура. + * Блок машинный, его форма меняется вместе с этим файлом, и жёсткий парсер + * ломался бы на каждой правке формулировки. + */ +export function materialAnchorsFrom(text) { + const body = String(text ?? ''); + const at = body.indexOf(ANCHOR_MARKER); + if (at < 0) return []; + return [...new Set(body.slice(at).match(/\b[0-9a-f]{40}\b/g) || [])]; +} + /** * Вердикт: `null` — все объявленные SHA существуют коммитами. * @@ -135,12 +149,29 @@ export function citedMaterialShas(text, headerLines = REVIEW_HEADER_LINES) { * * @param resolveReachable функция `(shas) => Map` */ -export function danglingMaterialRefusal(text, resolveReachable, headerLines = REVIEW_HEADER_LINES) { +export function danglingMaterialRefusal( + text, resolveReachable, headerLines = REVIEW_HEADER_LINES, resolveObjects = null, +) { const cited = citedMaterialShas(text, headerLines); if (!cited.length) return null; const refs = resolveReachable([...new Set(cited.map((item) => item.sha))]); const bad = cited.filter((item) => !refs.get(item.sha)); if (!bad.length) return null; + // Осиротевший SHA — ещё не потеря раунда, если якоря на месте (#414). Дерево + // и блобы адресуются содержимым: ребейз их не меняет, и материал находится + // командами из машинного блока. Отказ остаётся там, где не работает НИ ОДИН + // из объявленных способов найти материал. + const anchors = materialAnchorsFrom(text); + if (anchors.length && resolveObjects) { + const alive = anchors.filter((object) => resolveObjects(object)); + if (alive.length) { + return { warning: 'SHA раунда осиротел, но материал воспроизводим по якорям:' + + ` ${bad.map((item) => item.sha).join(', ')} недостижимы,` + + ` якорей живых ${alive.length} из ${anchors.length}.` + + ' Ребейз ветки после ревью — обычное дело; именно для этого якоря и' + + ' дописываются (#414).' }; + } + } const lines = bad .map((item) => ` строка ${item.line}: ${item.sha} → не достижим ни из одной ссылки origin`) .join('\n'); @@ -153,10 +184,106 @@ export function danglingMaterialRefusal(text, resolveReachable, headerLines = RE + ' а не значения, записанного до amend или rebase.'; } +/** Маркер машинного блока: по нему блок находится и заменяется целиком. */ +export const ANCHOR_MARKER = ''; + +/** + * Блок якорей материала — то, что переживает ребейз (#414). + * + * Зачем он, если SHA уже назван. SHA ветки — не свойство материала, а свойство + * истории, и история переписывается. На #403 спец-коммит переехал из + * `83005c3c` в `94502d3d` за пятнадцать минут до публикации отчёта: сообщение + * то же, содержимое то же, блоб ТЗ тот же (`56a92e12`), а команда из §2.10 + * `git diff 83005c3c..HEAD` через раунд не работала. Следующий ревьюер + * восстанавливал коммит по содержимому диффа руками. + * + * Дерево и блоб адресуются содержимым, поэтому ребейз их не меняет: пока текст + * где-нибудь достижим, найти его можно одной командой. Именно эти команды и + * пишутся в блок — отчёт обязан быть исполняемым, а не описательным. + * + * Блок машинный и помечен как машинный. Ревьюер его не заполняет: дисциплина + * ручного переписывания SHA здесь уже подвела, и заменять её другой ручной + * дисциплиной смысла нет. + */ +export function materialAnchorBlock({ sha, tree, branch, specs = [] } = {}) { + const short = (value) => (typeof value === 'string' ? value.slice(0, 12) : ''); + const lines = [ + ANCHOR_MARKER, + '', + '## Материал раунда', + '', + `- Ветка: \`${branch || 'dev'}\`, коммит \`${short(sha)}\` — ребейз его осиротит,` + + ' и это нормально: ниже якоря, которые ребейз не меняет.', + ]; + if (tree) { + lines.push(`- Дерево материала: \`${tree}\``); + lines.push(' ```'); + lines.push(` git log --all --format='%H %T' | grep ${short(tree)}`); + lines.push(' ```'); + } + for (const spec of specs) { + lines.push(`- ТЗ \`${spec.path}\`, блоб \`${spec.blob}\``); + lines.push(' ```'); + lines.push(` git log --all --find-object=${spec.blob} -- ${spec.path}`); + lines.push(' ```'); + } + if (!tree && !specs.length) { + lines.push('- Якоря снять не удалось: ветки задачи нет, материал читался по `dev`.'); + } + return `${lines.join('\n')}\n`; +} + +/** + * Разбор строки `--specs`: `blob путь;blob путь;`. + * + * Формат сырой намеренно: он рождается в `git ls-files -s` внутри workflow, и + * любая промежуточная сериализация здесь была бы лишним местом для ошибки. + */ +export function parseSpecList(raw) { + return String(raw ?? '') + .split(';') + .map((item) => item.trim()) + .filter(Boolean) + .map((item) => { + const [blob, ...rest] = item.split(/\s+/); + return { blob, path: rest.join(' ') }; + }) + .filter((item) => /^[0-9a-f]{40}$/.test(item.blob) && item.path); +} + +/** Дописать или заменить блок якорей в тексте документа. */ +export function withMaterialAnchors(text, anchors) { + const body = String(text ?? ''); + const at = body.indexOf(ANCHOR_MARKER); + const head = at >= 0 ? body.slice(0, at).replace(/\s+$/, '') : body.replace(/\s+$/, ''); + return `${head}\n\n---\n\n${materialAnchorBlock(anchors)}`; +} + const invokedDirectly = process.argv[1] && import.meta.url === new URL(`file://${process.argv[1]}`).href; if (invokedDirectly) { const argv = process.argv.slice(2); + // Режим дописывания якорей (#414): конвейер снял их при чтении материала. + const anchorArg = argv.find((item) => item.startsWith('--anchor=')); + if (anchorArg) { + const path = anchorArg.slice('--anchor='.length); + const value = (name) => { + const found = argv.find((item) => item.startsWith(`--${name}=`)); + return found ? found.slice(name.length + 3) : ''; + }; + const anchors = { + sha: value('sha'), + tree: value('tree'), + branch: value('branch'), + specs: parseSpecList(value('specs')), + }; + const text = readFileSync(path, 'utf8'); + writeFileSync(path, withMaterialAnchors(text, anchors), 'utf8'); + console.log(`якоря материала дописаны: дерево ${anchors.tree.slice(0, 12) || '—'},` + + ` ТЗ ${anchors.specs.length}`); + process.exit(0); + } + // Режим проверки объявленного материала (#413): на входе сам документ. const docArg = argv.find((item) => item.startsWith('--doc=')); if (docArg) { @@ -180,16 +307,25 @@ if (invokedDirectly) { } return map; }; - const refusal = danglingMaterialRefusal(text, resolveReachable); - if (refusal) { - console.error(`::error::${refusal.split('\n')[0]}`); - console.error(refusal); + const resolveObjects = (object) => spawnSync('git', ['cat-file', '-e', object], { + encoding: 'utf8', + }).status === 0; + const verdict = danglingMaterialRefusal( + text, resolveReachable, REVIEW_HEADER_LINES, resolveObjects, + ); + if (verdict && verdict.warning) { + console.log(`::warning::${verdict.warning}`); + } else if (verdict) { + console.error(`::error::${verdict.split('\n')[0]}`); + console.error(verdict); process.exit(1); } const cited = citedMaterialShas(text); - console.log(cited.length - ? `материал раунда объявлен и достижим с origin: ${cited.map((item) => item.sha).join(', ')}` - : 'материал раунда в шапке не объявлен — проверять нечего'); + if (!(verdict && verdict.warning)) { + console.log(cited.length + ? `материал раунда объявлен и достижим с origin: ${cited.map((item) => item.sha).join(', ')}` + : 'материал раунда в шапке не объявлен — проверять нечего'); + } process.exit(0); } const allowArg = argv.find((item) => item.startsWith('--allow=')); diff --git a/test/review-doc-guard.test.mjs b/test/review-doc-guard.test.mjs index c92f7c55..1d9eb925 100644 --- a/test/review-doc-guard.test.mjs +++ b/test/review-doc-guard.test.mjs @@ -3,7 +3,7 @@ import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; import { - REVIEW_DOC_ALLOWLIST, citedMaterialShas, danglingMaterialRefusal, pathsOutsideAllowlist, reviewDocPushRefusal, + ANCHOR_MARKER, REVIEW_DOC_ALLOWLIST, REVIEW_HEADER_LINES, citedMaterialShas, danglingMaterialRefusal, materialAnchorBlock, materialAnchorsFrom, parseSpecList, pathsOutsideAllowlist, reviewDocPushRefusal, withMaterialAnchors, } from '../scripts/review-doc-guard.mjs'; // #365. 28.08 шаг публикации ревью-дока запушил в dev коммит bb2919f с тридцатью @@ -137,3 +137,68 @@ test('шапка без объявления материала не судит assert.equal(danglingMaterialRefusal('# CODE-REVIEW-1-r1\n\nтекст\n', () => new Map()), null); assert.deepEqual(citedMaterialShas('# CODE-REVIEW-1-r1\n\nтекст\n'), []); }); + +// --- якоря, переживающие ребейз (#414) ------------------------------------- + +test('блок якорей содержит исполнимые команды, а не описание (#414)', () => { + const block = materialAnchorBlock({ + sha: '94502d3d67cacf85bdb9f69cd511b342989891fd', + tree: '3fc651fcb868eefa28755d01ec2b9377598dcb27', + branch: 'issue/403-area-relocation-safety', + specs: [{ + blob: '56a92e12dedc8fa541537ae5908dc6f1dfab43e8', + path: 'docs/specs/403-area-relocation-safety.md', + }], + }); + // Отчёт обязан быть исполняемым: на #403 канонная команда не работала, и + // следующий раунд восстанавливал коммит по содержимому диффа руками. + assert.match(block, /git log --all --find-object=56a92e12dedc8fa541537ae5908dc6f1dfab43e8/); + assert.match(block, /git log --all --format='%H %T' \| grep 3fc651fcb868/); + assert.match(block, /ребейз его осиротит/, 'блок обязан объяснять, зачем он нужен'); + assert.match(block, /material-anchors: сгенерировано конвейером/); +}); + +test('без ветки задачи блок честно говорит, что якорей нет (#414)', () => { + const block = materialAnchorBlock({ branch: '', sha: '', tree: '', specs: [] }); + assert.match(block, /Якоря снять не удалось/); +}); + +test('повторная приписка заменяет блок, а не копит его (#414)', () => { + const anchors = { sha: 'a'.repeat(40), tree: 'b'.repeat(40), branch: 'dev', specs: [] }; + const once = withMaterialAnchors('# отчёт\n\nтекст\n', anchors); + const twice = withMaterialAnchors(once, anchors); + assert.equal(twice.split(ANCHOR_MARKER).length - 1, 1, 'маркер обязан быть один'); + assert.match(twice, /# отчёт/); +}); + +test('список ТЗ разбирается и отсекает мусор (#414)', () => { + const parsed = parseSpecList( + `${'a'.repeat(40)} docs/specs/403-x.md;короткий docs/specs/y.md;${'b'.repeat(40)} ;`, + ); + assert.deepEqual(parsed, [{ blob: 'a'.repeat(40), path: 'docs/specs/403-x.md' }]); +}); + +test('осиротевший SHA при живых якорях — предупреждение, не отказ (#414)', () => { + const doc = withMaterialAnchors( + '- Материал: спец-файл на `HEAD = 83005c3c`\n', + { sha: 'c'.repeat(40), tree: 'd'.repeat(40), branch: 'issue/403-x', specs: [] }, + ); + const verdict = danglingMaterialRefusal( + doc, () => new Map([['83005c3c', null]]), REVIEW_HEADER_LINES, () => true, + ); + assert.ok(verdict.warning, 'раунд воспроизводим — ронять его нечего'); + assert.match(verdict.warning, /83005c3c/); + assert.match(verdict.warning, /по якорям/); +}); + +test('осиротевший SHA и мёртвые якоря — по-прежнему отказ (#414)', () => { + const doc = withMaterialAnchors( + '- Материал: спец-файл на `HEAD = 83005c3c`\n', + { sha: 'c'.repeat(40), tree: 'd'.repeat(40), branch: 'issue/403-x', specs: [] }, + ); + const verdict = danglingMaterialRefusal( + doc, () => new Map([['83005c3c', null]]), REVIEW_HEADER_LINES, () => false, + ); + assert.equal(typeof verdict, 'string'); + assert.match(verdict, /не достижим ни из одной ссылки origin/); +});