diff --git a/.github/workflows/process.yml b/.github/workflows/process.yml index cf99cfb2..deb7e897 100644 --- a/.github/workflows/process.yml +++ b/.github/workflows/process.yml @@ -745,6 +745,42 @@ jobs: fi echo "документ опубликован в $target: $doc" + # Материал раунда обязан быть достижим с origin (#413). + # + # SPEC-REVIEW-403-r2 объявил материал на `HEAD = 83005c3c`, и тот же SHA + # независимо назвал автор ТЗ в комментарии issue. Коммит существовал, но + # к моменту публикации был осиротевшим: ветку перебазировали за 15 минут + # ДО публикации документа, спец-коммит переехал в 94502d3d с тем же + # сообщением и тем же содержимым. Через раунд команда `git diff + # 83005c3c..HEAD` из §2.10 буквально не работала, и r3 восстанавливал + # реальный коммит по содержимому диффа руками. + # + # Проверка стоит ПОСЛЕ публикации намеренно. Артефакт ревью терялся здесь + # трижды (#171, #220), и «вердикт без документа» в этом репозитории + # дороже мёртвой ссылки: документ сначала спасается, потом судится. Шаг + # при этом идёт ДО «Переставить метку», поэтому инвариант «метка не + # сменилась = прогон упал» сохраняется. + # + # Достижимость считается от `refs/remotes/origin/*`, а не от локальных + # ссылок: осиротевший 83005c3c до сих пор лежит в клоне автора и + # достижим там из необновлённой локальной ветки. Читателю отчёта от этого + # пользы нет — он достанет только то, что есть на origin. + - name: "Материал раунда воспроизводим (#413)" + if: steps.rebase.outputs.conflict != 'true' + env: + NUM: ${{ github.event.issue.number }} + STAGE: ${{ needs.guard.outputs.stage }} + CYCLE: ${{ needs.guard.outputs.cycle }} + BRANCH: ${{ steps.branch.outputs.name }} + run: | + marker=CODE-REVIEW + if [ "$STAGE" = "spec" ]; then marker=SPEC-REVIEW; fi + doc="docs/reviews/${marker}-${NUM}-r${CYCLE}.md" + target="${BRANCH:-dev}" + git fetch -q origin "$target" + # Судится опубликованная версия, а не рабочая копия: именно её прочтёт + # следующий раунд. + git show "origin/$target:$doc" | node scripts/review-doc-guard.mjs --doc=- - name: Решение по вердикту id: decide if: steps.rebase.outputs.conflict != 'true' diff --git a/scripts/review-doc-guard.mjs b/scripts/review-doc-guard.mjs index f70d9e9c..32896a9c 100644 --- a/scripts/review-doc-guard.mjs +++ b/scripts/review-doc-guard.mjs @@ -23,6 +23,7 @@ * который пуш добавит в целевую ветку. Пустой список — тоже отказ: публиковать * нечего, значит что-то пошло не так раньше. */ +import { spawnSync } from 'node:child_process'; import { readFileSync } from 'node:fs'; export const REVIEW_DOC_ALLOWLIST = ['docs/reviews/']; @@ -59,10 +60,138 @@ export function reviewDocPushRefusal(paths, allowlist = REVIEW_DOC_ALLOWLIST) { + ' превратилось в «затереть dev целиком» (#365).'; } +/** + * Сколько первых строк документа считаются шапкой. Материал раунда объявляется + * там — измерено по корпусу: из 555 опубликованных ревью 409 называют SHA в + * первых пятнадцати строках. Дальше начинается проза, и в ней SHA упоминаются + * исторически («коммит bb2919f откатил dev»), проверять их нечего. + */ +export const REVIEW_HEADER_LINES = 20; + +/** Строки шапки, объявляющие материал раунда. */ +const MATERIAL_MARKER = /(Материал|Коммит дельты|SHA|HEAD\s*=|коммит)/; + +/** + * Кандидаты в SHA. Границы подобраны по корпусу, а не по вкусу: + * + * - 7–40 знаков: короче не бывает сокращений git, длиннее не бывает sha1. + * Отсекает заодно sha256 (64) — их в отчётах много, и они не коммиты; + * - хотя бы одна буква a–f: иначе в кандидаты попадают номера прогонов и даты + * вида `20260901`; + * - не после `#`: цвет `#607d8bff` — восемь шестнадцатеричных знаков; + * - не внутри более длинной шестнадцатеричной последовательности и не через + * дефис: `sha256-…` и обрезанные хвосты хешей кандидатами не считаются. + */ +const SHA_CANDIDATE = /(?..HEAD`. Проверять имеет смысл ровно + * то, что этой командой пользуются: объявление материала. Исторические + * упоминания в прозе — не обещание воспроизводимости. + */ +export function citedMaterialShas(text, headerLines = REVIEW_HEADER_LINES) { + const found = []; + String(text ?? '').split('\n').slice(0, headerLines).forEach((line, index) => { + if (!MATERIAL_MARKER.test(line)) return; + for (const sha of line.match(SHA_CANDIDATE) || []) { + if (!/[a-f]/.test(sha)) continue; + found.push({ line: index + 1, sha }); + } + }); + return found; +} + +/** + * Вердикт: `null` — все объявленные SHA существуют коммитами. + * + * Зачем этот рубеж (#413). `SPEC-REVIEW-403-r2.md` объявил материал раунда на + * `HEAD = 83005c3c`, и тот же SHA независимо назвал автор ТЗ в комментарии + * issue. Коммита с таким именем в репозитории нет и не было: клон не мелкий, + * `git rev-list --all` его не знает. Скорее всего значение снято до `amend` + * или `rebase` при публикации — то есть проверка `git rev-parse HEAD` перед + * выводом отчёта, которую требует §7.2, не выполнялась ни у автора, ни у + * ревьюера. + * + * Цена уже заплачена на следующем раунде: пункт «найти SHA, на котором получен + * предыдущий вердикт» выполнить буквально не удалось, реальный коммит + * реконструировали по содержимому диффа. + * + * Чего этот рубеж НЕ умеет, и это важно знать. Он судит момент публикации. + * Ветка задачи после ревью нередко перебазируется или сквошится, и SHA умирает + * уже потом — по корпусу таких объявлений 98 из 804. Здесь ловится другой + * класс: SHA, мёртвый уже в момент, когда его объявляют воспроизводимым. + * + * Достижимость проверяется от ссылок ПУБЛИКАЦИИ (`refs/remotes/origin/*` и + * теги), а не от локальных. Разница не теоретическая: осиротевший `83005c3c` + * до сих пор лежит объектом в клоне Codex и достижим там из локальной + * `refs/heads/issue/403-area-relocation-safety`, не обновлённой после ребейза. + * Читателю отчёта от этого нет никакой пользы — он может достать только то, + * что есть на origin. Локальная проверка дала бы «всё в порядке» ровно на той + * машине, где ошибку и совершили. + * + * @param resolveReachable функция `(shas) => Map` + */ +export function danglingMaterialRefusal(text, resolveReachable, headerLines = REVIEW_HEADER_LINES) { + 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; + const lines = bad + .map((item) => ` строка ${item.line}: ${item.sha} → не достижим ни из одной ссылки origin`) + .join('\n'); + return 'ревью-документ объявляет материал раунда на SHA, которого нет на' + + ` origin:\n${lines}\n` + + 'Команда `git diff ..HEAD` из PROCESS.md §2.10 на таком отчёте не' + + ' работает, а следующий раунд восстанавливает коммит по содержимому' + + ' диффа руками (#413). Сверьте SHA командой `git rev-parse HEAD`' + + ' непосредственно перед выводом отчёта — §7.2 требует именно этого,' + + ' а не значения, записанного до amend или rebase.'; +} + const invokedDirectly = process.argv[1] && import.meta.url === new URL(`file://${process.argv[1]}`).href; if (invokedDirectly) { const argv = process.argv.slice(2); + // Режим проверки объявленного материала (#413): на входе сам документ. + const docArg = argv.find((item) => item.startsWith('--doc=')); + if (docArg) { + const path = docArg.slice('--doc='.length); + let text; + try { + text = readFileSync(path === '-' ? 0 : path, 'utf8'); + } catch (error) { + console.error(`::error::ревью-документ не прочитан: ${path} (${error.code || error.message})`); + process.exit(1); + } + const resolveReachable = (shas) => { + const map = new Map(shas.map((sha) => [sha, null])); + for (const sha of shas) { + const probe = spawnSync('git', [ + 'for-each-ref', '--contains', sha, '--count=1', + '--format=%(refname)', 'refs/remotes/origin', 'refs/tags', + ], { encoding: 'utf8' }); + const ref = (probe.stdout || '').trim().split('\n')[0]; + if (probe.status === 0 && ref) map.set(sha, ref); + } + return map; + }; + const refusal = danglingMaterialRefusal(text, resolveReachable); + if (refusal) { + console.error(`::error::${refusal.split('\n')[0]}`); + console.error(refusal); + process.exit(1); + } + const cited = citedMaterialShas(text); + console.log(cited.length + ? `материал раунда объявлен и достижим с origin: ${cited.map((item) => item.sha).join(', ')}` + : 'материал раунда в шапке не объявлен — проверять нечего'); + process.exit(0); + } const allowArg = argv.find((item) => item.startsWith('--allow=')); const allowlist = allowArg ? allowArg.slice('--allow='.length).split(',').map((item) => item.trim()).filter(Boolean) diff --git a/test/review-doc-guard.test.mjs b/test/review-doc-guard.test.mjs index 8823cf63..c92f7c55 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, pathsOutsideAllowlist, reviewDocPushRefusal, + REVIEW_DOC_ALLOWLIST, citedMaterialShas, danglingMaterialRefusal, pathsOutsideAllowlist, reviewDocPushRefusal, } from '../scripts/review-doc-guard.mjs'; // #365. 28.08 шаг публикации ревью-дока запушил в dev коммит bb2919f с тридцатью @@ -87,3 +87,53 @@ test('шаг публикации в конвейере проверяет и и // Индексируется один путь, а не каталог. assert.match(step, /git add -- "\$doc"/); }); + +// --- материал раунда обязан быть достижим (#413) ---------------------------- + +test('SHA из шапки извлекаются, а из прозы — нет (#413)', () => { + const doc = [ + '# SPEC-REVIEW-403-r2', + '', + '## Скоуп', + '', + '- Материал: спец-файл на `HEAD = 83005c3c` (ветка `issue/403-x`,', + ' коммит «docs: revise area relocation safety spec»)', + '- Ревизия: 2', + ].join('\n') + '\n'.repeat(30) + 'Так коммит bb2919f7 откатил dev на три часа.\n'; + const cited = citedMaterialShas(doc); + assert.deepEqual(cited.map((item) => item.sha), ['83005c3c']); + assert.equal(cited[0].line, 5); +}); + +test('не-SHA в шапку не попадают: цвета, sha256, номера (#413)', () => { + const doc = [ + '- Материал: коммит `cbf5cc1b`, цвет #607d8bff, прогон 20260901,', + ' imageSha256 `9119ab87502038f787529f621c39e1e0d01f3bc3b0289051c3791a1886e97a6b`,', + ' ссылка sha256-abc1234def', + ].join('\n'); + assert.deepEqual(citedMaterialShas(doc).map((item) => item.sha), ['cbf5cc1b']); +}); + +test('недостижимый SHA останавливает раунд и объясняет, почему (#413)', () => { + const doc = '- Материал: спец-файл на `HEAD = 83005c3c`\n'; + const refusal = danglingMaterialRefusal(doc, () => new Map([['83005c3c', null]])); + assert.match(refusal, /83005c3c/); + assert.match(refusal, /не достижим ни из одной ссылки origin/); + // Отказ обязан называть и команду из канона, и способ не повторить: + // на #403 ревьюер снял HEAD до ребейза и не сверился перед выводом. + assert.match(refusal, /git diff/); + assert.match(refusal, /git rev-parse HEAD/); +}); + +test('достижимый SHA раунд не задерживает (#413)', () => { + const doc = '- Материал: коммит `cbf5cc1b`\n'; + const resolve = () => new Map([['cbf5cc1b', 'refs/remotes/origin/dev']]); + assert.equal(danglingMaterialRefusal(doc, resolve), null); +}); + +test('шапка без объявления материала не судится (#413)', () => { + // Часть документов материал не объявляет вовсе — по корпусу таких 146 из 555. + // Требовать объявление — отдельное решение о каноне, а не дело гейта. + assert.equal(danglingMaterialRefusal('# CODE-REVIEW-1-r1\n\nтекст\n', () => new Map()), null); + assert.deepEqual(citedMaterialShas('# CODE-REVIEW-1-r1\n\nтекст\n'), []); +});