From 23d6681d3e7218d8961de0dfe87691444dd1f657 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 10:38:03 +0300 Subject: [PATCH] =?UTF-8?q?ci:=20=D0=BF=D1=83=D0=B1=D0=BB=D0=B8=D0=BA?= =?UTF-8?q?=D0=B0=D1=86=D0=B8=D1=8F=20=D1=80=D0=B5=D0=B2=D1=8C=D1=8E-?= =?UTF-8?q?=D0=B4=D0=BE=D0=BA=D0=B0=20=D0=BD=D0=B5=20=D0=B8=D0=BC=D0=B5?= =?UTF-8?q?=D0=B5=D1=82=20=D0=BF=D1=80=D0=B0=D0=B2=D0=B0=20=D1=82=D1=80?= =?UTF-8?q?=D0=BE=D0=B3=D0=B0=D1=82=D1=8C=20=D0=BD=D0=B8=D1=87=D0=B5=D0=B3?= =?UTF-8?q?=D0=BE,=20=D0=BA=D1=80=D0=BE=D0=BC=D0=B5=20=D0=B4=D0=BE=D0=BA?= =?UTF-8?q?=D1=83=D0=BC=D0=B5=D0=BD=D1=82=D0=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 28.08 коммит bb2919f уехал в dev с тридцатью файлами вместо одного markdown: откатил отревьюженную реализацию #359, вернул старые чанки, оставил в dist/ двойной набор. dev держал откаченное дерево три часа. Сообщение коммита было невинным, и от рутины инцидент отличался только диффом. Механизм воспроизведён локально, а не предположен. `git checkout -- .` восстанавливает рабочее дерево ИЗ ИНДЕКСА, `git clean -fd` убирает неотслеживаемое — ни то, ни другое индекс не трогает. Ревьюер работает с Bash и, проверяя «умеет ли тест падать», вполне может сделать git add; всё оставшееся у него в индексе прежняя уборка сохраняла, и следующий git commit забирал это вместе с документом. Отсюда три рубежа, каждый закрывает свой отрезок пути. База: reset --hard на свежий origin/$target снимает и индекс, и дерево разом. Терять нечего — документ приезжает из RUNNER_TEMP, а не из рабочей копии. Индексируется ровно один путь, а не каталог. Индекс: перед коммитом дифф проверяется allowlist'ом docs/reviews/. Диапазон: перед КАЖДЫМ push проверяется origin/$target...HEAD — то есть то, что пуш добавит в ветку. Проверок две, потому что push делается из двух мест, и второй путь срабатывает ровно тогда, когда dev ушёл вперёд — в тех самых условиях, при которых случился bb2919f. Пустой дифф — тоже отказ: публиковать нечего означает, что документа нет, а прежняя редакция шага выходила тут с нулём и оставляла вердикт без артефакта (#171). Сравнение по префиксу каталога, а не подстрокой: docs/reviews-old и docs/reviewsx разрешёнными не считаются. Форс-пуш отсутствует и закреплён тестом. Четыре мутанта проверены руками, два добавлены в реестр. Пятый — «убрать одну из двух проверок диапазона» — сначала выжил: тест требовал наличия, а не количества. Тест усилен до подсчёта, мутант убит. Issue: #365 User-Visible: no --- .github/workflows/process.yml | 51 ++++++++++++++----- scripts/mutation-gate.mjs | 22 +++++++++ scripts/review-doc-guard.mjs | 79 ++++++++++++++++++++++++++++++ test/review-doc-guard.test.mjs | 89 ++++++++++++++++++++++++++++++++++ 4 files changed, 229 insertions(+), 12 deletions(-) create mode 100644 scripts/review-doc-guard.mjs create mode 100644 test/review-doc-guard.test.mjs diff --git a/.github/workflows/process.yml b/.github/workflows/process.yml index 21a1dcf9..cf99cfb2 100644 --- a/.github/workflows/process.yml +++ b/.github/workflows/process.yml @@ -642,14 +642,31 @@ jobs: marker=CODE-REVIEW if [ "$STAGE" = "spec" ]; then marker=SPEC-REVIEW; fi doc="docs/reviews/${marker}-${NUM}-r${CYCLE}.md" - # Рабочая копия отбрасывается ДО того, как документ попадёт в дерево: - # ревьюер правит код, проверяя «умеет ли тест падать», и его правки - # публиковаться не должны. - git checkout -- . 2>/dev/null || true - # docs/reviews исключён из уборки: ревьюер мог написать документ по - # старому пути, и клин не должен его съесть до `git add` — ровно так - # оба пути остаются работоспособными. - git clean -fd -e docs/reviews -e node_modules >/dev/null 2>&1 || true + # Документ спасается ПЕРВЫМ делом. Ревьюер мог написать его по старому + # пути прямо в рабочую копию, а дальше эта копия будет отброшена + # целиком — и вместе с ней пропал бы артефакт (#220). + if [ ! -f "$SOURCE" ] && [ -f "$doc" ]; then + cp "$doc" "$SOURCE" + echo "документ найден в рабочей копии и сохранён в $SOURCE" + fi + # Reset, а не checkout+clean, и вот почему (#365). + # + # 28.08 коммит bb2919f уехал в dev с тридцатью файлами вместо одного + # markdown: откатил отревьюженную реализацию #359, вернул старые чанки + # и держал dev откаченным три часа. Механизм воспроизведён: + # `git checkout -- .` восстанавливает рабочее дерево ИЗ ИНДЕКСА, а + # `git clean -fd` убирает неотслеживаемое — ни то, ни другое индекс не + # трогает. Ревьюер работает с Bash и в ходе проверки «умеет ли тест + # падать» вполне может сделать `git add`; всё, что осталось у него в + # индексе, прежняя уборка сохраняла, и следующий же `git commit` + # забирал это вместе с документом. Сообщение при этом невинное, и от + # рутины инцидент отличается только диффом. + # + # `reset --hard` снимает и индекс, и дерево разом. Терять нечего: + # документ приезжает извне репозитория, из RUNNER_TEMP. + git fetch -q origin "$target" + git reset -q --hard "origin/$target" + git clean -fdq -e node_modules >/dev/null 2>&1 || true # Документ приезжает извне репозитория (#220). Три раунда подряд он # терялся, пока лежал некоммитнутым файлом в том же дереве, которое # ревьюер мутирует и затем восстанавливает: `git checkout -- .` плюс @@ -661,11 +678,12 @@ jobs: cp "$SOURCE" "$doc" echo "документ взят из $SOURCE ($(wc -c < "$doc") байт)" else - # Совместимость: ревьюер мог написать по старому пути, если промпт - # ещё не обновился в этой ветке. - echo "::warning::$SOURCE не найден — ищу документ в рабочей копии" + echo "::warning::$SOURCE не найден — документа для публикации нет" fi - git add docs/reviews 2>/dev/null || true + # Индексируется ровно один путь, а не каталог: `git add docs/reviews` + # забрал бы всё, что там окажется, а после reset там не должно быть + # ничего постороннего — но полагаться на «не должно» здесь нельзя. + git add -- "$doc" 2>/dev/null || true if git diff --cached --quiet; then # Пустая рабочая копия — ещё не провал: ревьюер иногда коммитит # документ сам, своим app-токеном мимо этого шага (CODE-REVIEW-150-r1, @@ -683,6 +701,8 @@ jobs: echo "::error::вердикт есть, а документа нет: ни $SOURCE, ни $doc в рабочей копии, ни $doc в $target — ревью без артефакта (#171, #220)" exit 1 fi + # Первый рубеж: что вообще проиндексировано. + git diff --cached --name-only | node scripts/review-doc-guard.mjs git -c user.name="claude[bot]" \ -c user.email="209825114+claude[bot]@users.noreply.github.com" \ commit -q -F - < (item.endsWith('/') ? item : `${item}/`));", + replace: " const prefixes = allowlist.map((item) => item.replace(/\\/$/, ''));", + }], + }, + { + id: 'review-doc-guard-allows-empty-diff', + guard: 'node --test --test-name-pattern="пустой дифф" test/review-doc-guard.test.mjs', + because: 'пустой дифф означает, что документа нет: прежняя редакция шага выходила тут с ' + + 'нулём, и вердикт ревью оставался без артефакта (#171, #365)', + patches: [{ + file: 'scripts/review-doc-guard.mjs', + find: ' if (!cleaned.length) {', + replace: ' if (false) {', + }], + }, { id: 'no-new-any-judges-every-line', guard: 'node --test --test-name-pattern="нетронутой строке гейт не блокирует" ' diff --git a/scripts/review-doc-guard.mjs b/scripts/review-doc-guard.mjs new file mode 100644 index 00000000..f70d9e9c --- /dev/null +++ b/scripts/review-doc-guard.mjs @@ -0,0 +1,79 @@ +#!/usr/bin/env node +/** + * Публикация ревью-документа не имеет права трогать ничего, кроме него (#365). + * + * git diff --name-only "origin/dev...HEAD" | node scripts/review-doc-guard.mjs + * node scripts/review-doc-guard.mjs --allow 'docs/specs/' < paths.txt + * + * Что случилось. 28.08 шаг публикации запушил в `dev` коммит `bb2919f` с + * тридцатью файлами вместо одного markdown: откатил отревьюженную реализацию + * #359, вернул старые чанки и оставил в `dist/` двойной набор. `dev` держал + * откаченное дерево три часа, пока владелец не восстановил его руками + * (`fd762fa`). Сообщение коммита при этом было невинным — «docs: review document + * for #359», — и от рутины инцидент отличался только диффом. + * + * Почему это класс, а не случай. Пушащий шаг ничем не ограничен по путям, а его + * рабочая копия может разойтись с origin по десятку причин: гонка параллельных + * агентов за `dev` (в тот вечер их было три), ревью длиной в сорок минут, + * ветка задачи, которой нет. Любой такой рассинхрон превращает «положить один + * markdown» в «затереть dev целиком», и заметить это может только аудит дельты. + * Релиз собирается из `dev` — рецидив уехал бы пользователям. + * + * Поэтому проверка судит не намерение шага, а его результат: набор путей, + * который пуш добавит в целевую ветку. Пустой список — тоже отказ: публиковать + * нечего, значит что-то пошло не так раньше. + */ +import { readFileSync } from 'node:fs'; + +export const REVIEW_DOC_ALLOWLIST = ['docs/reviews/']; + +/** + * Пути вне разрешённых каталогов. + * + * Сравнение по префиксу каталога, а не по расширению: `docs/reviews/x.md` + * разрешён, `docs/reviews-old/x.md` — нет, потому что префикс каталога + * заканчивается слэшем и подстрокой не притворяется. + */ +export function pathsOutsideAllowlist(paths, allowlist = REVIEW_DOC_ALLOWLIST) { + const prefixes = allowlist.map((item) => (item.endsWith('/') ? item : `${item}/`)); + return [...new Set((paths || []) + .map((line) => String(line).trim()) + .filter(Boolean))] + .filter((path) => !prefixes.some((prefix) => path.startsWith(prefix))) + .sort(); +} + +/** Вердикт по набору путей: `null` — можно публиковать. */ +export function reviewDocPushRefusal(paths, allowlist = REVIEW_DOC_ALLOWLIST) { + const cleaned = [...new Set((paths || []).map((line) => String(line).trim()).filter(Boolean))]; + if (!cleaned.length) { + return 'публиковать нечего: дифф пуст, а шаг вызван — значит документ не создан' + + ' либо база уже содержит его'; + } + const outside = pathsOutsideAllowlist(cleaned, allowlist); + if (!outside.length) return null; + return `публикация ревью-документа задевает ${outside.length} путь(ей) вне` + + ` ${allowlist.join(', ')}:\n ${outside.join('\n ')}\n` + + 'Пуш отменён. Так 28.08 коммит bb2919f откатил dev на три часа:' + + ' рабочая копия шага разошлась с origin, и «положить один markdown»' + + ' превратилось в «затереть dev целиком» (#365).'; +} + +const invokedDirectly = process.argv[1] + && import.meta.url === new URL(`file://${process.argv[1]}`).href; +if (invokedDirectly) { + const argv = process.argv.slice(2); + const allowArg = argv.find((item) => item.startsWith('--allow=')); + const allowlist = allowArg + ? allowArg.slice('--allow='.length).split(',').map((item) => item.trim()).filter(Boolean) + : REVIEW_DOC_ALLOWLIST; + const paths = readFileSync(0, 'utf8').split('\n'); + const refusal = reviewDocPushRefusal(paths, allowlist); + if (refusal) { + console.error(`::error::${refusal.split('\n')[0]}`); + console.error(refusal); + process.exit(1); + } + const count = paths.map((line) => line.trim()).filter(Boolean).length; + console.log(`дифф публикации чист: ${count} файл(ов), все в ${allowlist.join(', ')}`); +} diff --git a/test/review-doc-guard.test.mjs b/test/review-doc-guard.test.mjs new file mode 100644 index 00000000..8823cf63 --- /dev/null +++ b/test/review-doc-guard.test.mjs @@ -0,0 +1,89 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; + +import { + REVIEW_DOC_ALLOWLIST, pathsOutsideAllowlist, reviewDocPushRefusal, +} from '../scripts/review-doc-guard.mjs'; + +// #365. 28.08 шаг публикации ревью-дока запушил в dev коммит bb2919f с тридцатью +// файлами вместо одного markdown: откатил отревьюженную реализацию #359, вернул +// старые чанки, оставил в dist/ двойной набор. dev держал откаченное дерево три +// часа. Сообщение коммита было невинным — «docs: review document for #359», — и +// от рутины инцидент отличался только диффом. Релиз собирается из dev. + +test('чистая публикация проходит (#365 AC1)', () => { + assert.equal(reviewDocPushRefusal(['docs/reviews/CODE-REVIEW-359-r1.md']), null); + assert.equal(reviewDocPushRefusal([ + 'docs/reviews/SPEC-REVIEW-1-r1.md', 'docs/reviews/SPEC-REVIEW-1-r2.md', + ]), null); +}); + +test('посторонний путь отменяет пуш и называет файлы (#365 AC2)', () => { + const refusal = reviewDocPushRefusal([ + 'docs/reviews/CODE-REVIEW-359-r1.md', + 'src/houseplan-card.ts', + 'dist/houseplan-card.js', + ]); + assert.match(refusal, /задевает 2 путь\(ей\)/); + assert.match(refusal, /dist\/houseplan-card\.js/); + assert.match(refusal, /src\/houseplan-card\.ts/); + // Причина названа, а не только факт: без неё следующий читатель решит, что + // проверка придирается, и снимет её. + assert.match(refusal, /bb2919f/); +}); + +test('пустой дифф — тоже отказ, а не тихий успех (#365)', () => { + // Публиковать нечего означает, что что-то пошло не так раньше. Прежняя + // редакция шага в таком случае выходила с нулём, и вердикт ревью оставался + // без артефакта (#171). + assert.match(reviewDocPushRefusal([]), /публиковать нечего/); + assert.match(reviewDocPushRefusal(['', ' ']), /публиковать нечего/); +}); + +test('соседний каталог с похожим именем не считается разрешённым (#365)', () => { + // Сравнение по префиксу каталога со слэшем: docs/reviews-old подстрокой не + // притворяется. + assert.deepEqual( + pathsOutsideAllowlist(['docs/reviews-old/x.md', 'docs/reviews/y.md']), + ['docs/reviews-old/x.md'], + ); + assert.deepEqual(pathsOutsideAllowlist(['docs/reviewsx.md']), ['docs/reviewsx.md']); +}); + +test('allowlist задаётся снаружи и по умолчанию только docs/reviews (#365)', () => { + assert.deepEqual(REVIEW_DOC_ALLOWLIST, ['docs/reviews/']); + assert.equal(reviewDocPushRefusal(['docs/specs/1.md'], ['docs/specs']), null); + assert.match(reviewDocPushRefusal(['docs/specs/1.md']), /docs\/specs\/1\.md/); +}); + +test('шаг публикации в конвейере проверяет и индекс, и то, что уедет (#365 AC4)', () => { + const workflow = readFileSync( + new URL('../.github/workflows/process.yml', import.meta.url), 'utf8', + ); + const step = workflow.slice( + workflow.indexOf('- name: Опубликовать документ ревью'), + workflow.indexOf('- name: Решение по вердикту'), + ); + assert.ok(step.length > 500, 'шаг публикации не найден'); + // Два рубежа: что проиндексировано и что пуш добавит в ветку. Расходились они + // именно тогда, когда база оказывалась не той. + assert.equal( + (step.match(/git diff --cached --name-only \| node scripts\/review-doc-guard\.mjs/g) || []).length, + 1, 'индекс проверяется один раз, перед коммитом', + ); + // Дважды: push делается из двух мест — сразу и после ребейза при гонке. Одна + // проверка на два пути означала бы, что второй путь не проверен вовсе, а + // именно он срабатывает, когда dev ушёл вперёд — то есть в тех самых + // условиях, при которых случился bb2919f. + assert.equal( + (step.match(/git diff --name-only "origin\/\$target\.\.\.HEAD" \| node scripts\/review-doc-guard\.mjs/g) || []).length, + 2, 'диапазон проверяется перед каждым push', + ); + // Свежая база вместо той, что лежала здесь сорок минут назад. + assert.match(step, /git reset -q --hard "origin\/\$target"/); + // Форс-пуш запрещён: ветка двигается только вперёд. + assert.equal(/--force/.test(step), false, 'в публикации ревью-дока не должно быть force-push'); + // Индексируется один путь, а не каталог. + assert.match(step, /git add -- "\$doc"/); +});