mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
ci: anchor the review material to content, not to history
#413 закрыл класс «SHA мёртв уже в момент публикации». Остаётся более частый: SHA был жив, а умер потом — по корпусу таких объявлений 98 из 804, потому что ветку задачи после ревью перебазируют, сквошат или удаляют. SHA коммита — свойство истории, а история переписывается. Содержимое не переписывается: git адресует деревья и блобы их хешем. На #403 спец-коммит переехал из 83005c3c в94502d3d, а блоб ТЗ у обоих один — 56a92e12; по нему материал находится одной командой независимо от ребейза. Конвейер снимает якоря там, где читает материал — в шаге перехода на ветку задачи, пока рабочая копия равна тому, что прочтёт ревьюер. В шаге публикации спрашивать поздно: дерево уже сброшено на целевую ветку. При публикации якоря дописываются машинным блоком: дерево материала и блоб каждого ТЗ, каждый со своей исполнимой командой поиска. Блок машинный и помечен как машинный. Ревьюер его не заполняет: дисциплина ручного переписывания SHA здесь уже подвела, и заменять её другой ручной дисциплиной смысла нет. Гейт #413 смягчён ровно там, где обязан: осиротевший SHA при живых якорях — предупреждение, а не отказ. Ронять раунд, который воспроизводим, было бы той же ошибкой в другую сторону. Отказ остаётся, когда не работает ни один объявленный способ найти материал. Проверено на настоящем осиротевшем случае: блок, собранный для94502d3d, находит и дерево, и блоб ТЗ; тот же документ с якорями даёт предупреждение вместо отказа, без якорей — отказ. Issue: #416 User-Visible: no
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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<sha, ref|null>`
|
||||
*/
|
||||
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 = '<!-- material-anchors: сгенерировано конвейером (#414) -->';
|
||||
|
||||
/**
|
||||
* Блок якорей материала — то, что переживает ребейз (#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='));
|
||||
|
||||
@@ -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/);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user