mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-03 21:28:59 +00:00
ci: refuse a review round that cites an unreachable SHA
SPEC-REVIEW-403-r2 объявил материал раунда на `HEAD = 83005c3c`, и тот же SHA независимо назвал автор ТЗ в комментарии issue. Разбор подтвердил находку и уточнил её: коммит существовал, но к моменту публикации был осиротевшим. Ветку перебазировали за пятнадцать минут ДО публикации документа — спец-коммит переехал в94502d3dс тем же сообщением и тем же содержимым (блоб ТЗ у обоих56a92e12). Через раунд команда `git diff 83005c3c..HEAD` из §2.10 буквально не работала, и r3 восстанавливал коммит по содержимому диффа руками. Гейт судит только объявление материала в шапке документа, а не каждое шестнадцатеричное слово: в прозе SHA упоминаются исторически, и обещания воспроизводимости на них нет. Границы кандидата подобраны по корпусу — 7–40 знаков, хотя бы одна буква, не после `#`, не внутри длинного хеша; это отсекает sha256, цвета и номера прогонов. Достижимость считается от refs/remotes/origin, а не от локальных ссылок. Разница не теоретическая: осиротевший 83005c3c до сих пор достижим в клоне автора из необновлённой локальной ветки — локальная проверка сказала бы «всё в порядке» ровно на той машине, где ошибку и совершили. Шаг стоит ПОСЛЕ публикации и ДО перестановки метки. Артефакт ревью терялся здесь трижды (#171, #220), и «вердикт без документа» дороже мёртвой ссылки: документ сначала спасается, потом судится. Инвариант «метка не сменилась = прогон упал» при этом сохраняется. Проверено на настоящих документах: SPEC-REVIEW-403-r2 отказ, CODE-REVIEW-390-r1 проходит, документ без объявления материала не судится. Issue: #413 User-Visible: no
This commit is contained in:
@@ -745,6 +745,42 @@ jobs:
|
|||||||
fi
|
fi
|
||||||
echo "документ опубликован в $target: $doc"
|
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: Решение по вердикту
|
- name: Решение по вердикту
|
||||||
id: decide
|
id: decide
|
||||||
if: steps.rebase.outputs.conflict != 'true'
|
if: steps.rebase.outputs.conflict != 'true'
|
||||||
|
|||||||
@@ -23,6 +23,7 @@
|
|||||||
* который пуш добавит в целевую ветку. Пустой список — тоже отказ: публиковать
|
* который пуш добавит в целевую ветку. Пустой список — тоже отказ: публиковать
|
||||||
* нечего, значит что-то пошло не так раньше.
|
* нечего, значит что-то пошло не так раньше.
|
||||||
*/
|
*/
|
||||||
|
import { spawnSync } from 'node:child_process';
|
||||||
import { readFileSync } from 'node:fs';
|
import { readFileSync } from 'node:fs';
|
||||||
|
|
||||||
export const REVIEW_DOC_ALLOWLIST = ['docs/reviews/'];
|
export const REVIEW_DOC_ALLOWLIST = ['docs/reviews/'];
|
||||||
@@ -59,10 +60,138 @@ export function reviewDocPushRefusal(paths, allowlist = REVIEW_DOC_ALLOWLIST) {
|
|||||||
+ ' превратилось в «затереть dev целиком» (#365).';
|
+ ' превратилось в «затереть 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 = /(?<![0-9a-f#-])[0-9a-f]{7,40}(?![0-9a-f-])/g;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* SHA, объявленные материалом раунда: `[{ line, sha }]`.
|
||||||
|
*
|
||||||
|
* Зачем отдельная функция и почему только шапка. PROCESS.md §2.10 требует
|
||||||
|
* называть SHA предыдущего раунда затем, чтобы дельта следующего объявлялась
|
||||||
|
* воспроизводимой командой `git diff <sha>..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<sha, ref|null>`
|
||||||
|
*/
|
||||||
|
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 <sha>..HEAD` из PROCESS.md §2.10 на таком отчёте не'
|
||||||
|
+ ' работает, а следующий раунд восстанавливает коммит по содержимому'
|
||||||
|
+ ' диффа руками (#413). Сверьте SHA командой `git rev-parse HEAD`'
|
||||||
|
+ ' непосредственно перед выводом отчёта — §7.2 требует именно этого,'
|
||||||
|
+ ' а не значения, записанного до amend или rebase.';
|
||||||
|
}
|
||||||
|
|
||||||
const invokedDirectly = process.argv[1]
|
const invokedDirectly = process.argv[1]
|
||||||
&& import.meta.url === new URL(`file://${process.argv[1]}`).href;
|
&& import.meta.url === new URL(`file://${process.argv[1]}`).href;
|
||||||
if (invokedDirectly) {
|
if (invokedDirectly) {
|
||||||
const argv = process.argv.slice(2);
|
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 allowArg = argv.find((item) => item.startsWith('--allow='));
|
||||||
const allowlist = allowArg
|
const allowlist = allowArg
|
||||||
? allowArg.slice('--allow='.length).split(',').map((item) => item.trim()).filter(Boolean)
|
? allowArg.slice('--allow='.length).split(',').map((item) => item.trim()).filter(Boolean)
|
||||||
|
|||||||
@@ -3,7 +3,7 @@ import assert from 'node:assert/strict';
|
|||||||
import { readFileSync } from 'node:fs';
|
import { readFileSync } from 'node:fs';
|
||||||
|
|
||||||
import {
|
import {
|
||||||
REVIEW_DOC_ALLOWLIST, pathsOutsideAllowlist, reviewDocPushRefusal,
|
REVIEW_DOC_ALLOWLIST, citedMaterialShas, danglingMaterialRefusal, pathsOutsideAllowlist, reviewDocPushRefusal,
|
||||||
} from '../scripts/review-doc-guard.mjs';
|
} from '../scripts/review-doc-guard.mjs';
|
||||||
|
|
||||||
// #365. 28.08 шаг публикации ревью-дока запушил в dev коммит bb2919f с тридцатью
|
// #365. 28.08 шаг публикации ревью-дока запушил в dev коммит bb2919f с тридцатью
|
||||||
@@ -87,3 +87,53 @@ test('шаг публикации в конвейере проверяет и и
|
|||||||
// Индексируется один путь, а не каталог.
|
// Индексируется один путь, а не каталог.
|
||||||
assert.match(step, /git add -- "\$doc"/);
|
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'), []);
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user