12 KiB
Код-ревью #706 — заход r1
Материал ревью: origin/dev...HEAD, вершина ae0e516af16e2ebcea9642949135cb950ce65eb3
(один коммит поверх dev@e7fca7b9). Трек show, мутанты по диффу на материале не
запрашивались (не находка).
Скоуп
Конвейер перестаёт снимать S7-code-review без возврата, когда исход rereview
после точного кандидата (#492) возвращает задачу в ту же метку, из которой она
пришла: FROM == TO. Совмещённый вызов gh issue edit --add-label X --remove-label X
добавлял и тут же снимал одну метку — итог был «без статуса», событие labeled не
приходило, новый заход ревью не стартовал (см. инцидент #699, зафиксированный в теле
issue).
Правка вводит scripts/status-label.mjs (moveStatusLabel): при FROM == TO метка
снимается и ставится заново двумя вызовами gh через relabel автосверки (#555,
scripts/process-reconcile.mjs:342) — одна попытка восстановления, затем исключение;
при разных метках — как раньше, одной правкой. Шаг «Переставить метку» в
_process.yml вызывает скрипт вместо инлайн gh issue edit. PROCESS.md описывает
новое поведение точного кандидата. Три AC из тела issue закрыты тестами
test/status-label.test.mjs и двумя мутантами в scripts/mutation-registry.mjs.
Класса A (публичный API/контракт) в диффе нет — только скрипты процесса и workflow.
Как проверялось
Дешёвые гейты на этом SHA уже подтверждены (#343): Validate ae0e516a — success
(https://github.com/Matysh/houseplan-card/actions/runs/36521621591). tsc --noEmit,
npm test, npm run build на этом прогоне не перегонял; вместо этого:
- прочитан код
scripts/status-label.mjs,.github/workflows/_process.yml(весь шаг «Переставить метку» и его окружение),PROCESS.md(правило точного кандидата),scripts/process-reconcile.mjs:342-351(relabel, на которую опирается новый скрипт); node --test --test-name-pattern="#706" test/status-label.test.mjs— 4/4 зелёных;- оба новых мутанта применены руками к копии дерева (
/tmp, не в рабочей копии) и проверено, что целевой юнит-тест красится:rereview-relabel-in-one-call(патчscripts/status-label.mjs,if (from === to)→if (false)) — тест#706 rereviewпадает:expected: 'relabeled', actual: 'moved';process-label-step-combined-again(откат шага на инлайнgh issue edit) — тест#706 шаг конвейерападает наassert.doesNotMatch(.../gh issue edit/);
node scripts/smoke-select.mjs --base origin/dev --head HEAD— «Исполняемого frontend-диффа нет (src/**/*.tsне тронут)», браузерные смоки не выбираются — верно: правка не трогаетsrc/**;python3 -c "import yaml; yaml.safe_load(open('.github/workflows/_process.yml'))"— YAML синтаксически валиден после правки шага;node --check scripts/status-label.mjs— синтаксис ОК;- прослежена совместимость сигнатур: локальный
gh(args, {allowFailure})вstatus-label.mjsвызываетсяrelabel(repo, {number: issue}, to, execute)изprocess-reconcile.mjsтеми же двумя аргументами (execute(args)иexecute(args, {allowFailure: true})) — контракт совпадает; - проверено, что заявленный
cancel-in-progress: falseв группеconcurrency: process-issue-${{ github.event.issue.number }}действительно стоит на всех четырёх job (_process.yml:53-55, 317-319, 974-976, 1350-1352) — довод автора «повторныйlabeledне отменяет текущий прогон, а встаёт в очередь» подтверждён; без этого перестановка метки из середины ещё выполняющегося workflow-прогона могла бы породить гонку; - проверено, что комбинация «одна и та же метка на обоих концах» больше нигде в
_process.ymlне встречается кроме исправленного места (grepпоadd-label.*remove-label): строки 462, 702, 822 — везде разные метки на входе и выходе, тот же класс бага там не воспроизводится, latent-дефект не пропущен; - запущен полный
npm run gate:smallв фоне как избыточная перепроверка (дублирует уже зелёный Validate) — не блокировал вывод вердикта; на момент публикации документа ещё выполнялся, к находкам не привёл ни на одном пройденном шаге.
AC · чем доказан · чем краснеет
| AC | Доказательство | Чем краснеет | Проверено |
|---|---|---|---|
AC1: FROM == TO → снятие, затем постановка, задача остаётся в S7-code-review |
#706 rereview |
rereview-relabel-in-one-call |
тест зелёный; мутант применён вручную — тест краснеет (см. выше) |
AC2: FROM ≠ TO → одна правка; без FROM — только постановка |
#706 обычный исход |
— (поведение до #706, регрессии не требуется) | тест зелёный, прочитан код ветки moved |
AC3: сбой повторной постановки роняет шаг; в шаге нет совмещённого gh issue edit |
#706 сбой…, #706 шаг конвейера |
process-label-step-combined-again (для второй половины); восстановление — мутанты relabel из #555 |
тесты зелёные; мутант применён вручную — тест краснеет (см. выше) |
Все три AC — защитные (гард против «задача осталась без статуса молча»), у каждого есть строка «чем доказан/чем краснеет» с результатом прогона, пустых столбцов нет.
Находки
Не найдено. Синтаксис аргументов CLI (arg() в status-label.mjs) проверен вручную
на всех четырёх флагах (--repo=, --issue=, --from=, --to=) — смещение среза
name.length + 3 совпадает с длиной префикса --{name}= для каждого; ошибки на
пустом --from= (случай без исходной метки) не возникает, т.к. пустая строка — валидный
случай по AC2.
Что проверено и корректно
moveStatusLabelв обычном случае (FROM ≠ TO) даёт байт-в-байт то же поведение, что и прежний инлайнgh issue edit --add-label TO --remove-label FROM— регрессии для всех исходов, кромеrereview, нет.- Обработка ошибок сохраняет прежнюю семантику шага: неудача любого
gh-вызова бросает исключение →console.error('::error::...')→process.exit(1)→ шаг падает → срабатывает последующийif: failure()(«Позвать владельца»), как и раньше при падении инлайн-команды. PROCESS.md(§ про точного кандидата) обновлён в том же диффе и согласован с кодом: описывает именно снятие-и-постановку при разошедшемся patch-id, а не общий случай.- User-Visible-трейлер не требуется и не заявлен — правка невидима пользователю карточки (внутренний процесс ревью), changelog не тронут — это ожидаемо.
- Тесты корректно чувствительны:
#706 шаг конвейерапроверяет и наличие вызова скрипта, и отсутствие возврата кgh issue edit— не только позитивную, но и негативную сторону структуры шага.
Чего не проверял
- Живой прогон
rereviewна самом конвейере — по признанию автора, воспроизводится только при следующем слиянии со сменой patch-id; не гейт этого ревью (сценарий по своей природе недоступен до реального конкурентного слияния). npm run gate:small,process-gate --range,mutation-gate --check, диф-мутанты по 6 шардам (162/162) — заявлены автором, не перегонял;tsc --noEmit,npm test,npm run buildпокрыты цитированным зелёным Validate на этом SHA и повторно не гонялись согласно правилу дешёвых гейтов;npm run gate:smallзапущен фоново как необязательная подстраховка, не как обязательный гейт этого раунда.actionlint— бинарь недоступен в этом окружении; синтаксис шага проверен вручную построчным сравнением сfind/replaceмутанта и YAML-парсером Python, но actionlint-специфичные проверки (например, контекстsteps.*.outputsв других частях файла) не перегонялись.- Golden, pytest, инварианты модели — диф не трогает
src/**, рендер, геометрию или Python-бэкенд; по правилам объёма гейтов не применимы, что подтвержденоsmoke-select.mjs, а не только предположением.
Вердикт
Зелёный. Все три AC доказаны тестами, которые я проверил на способность падать (применил оба мутанта вручную к копии дерева). Класса A нет, User-Visible не требуется, изменение локально и соразмерно заявленной сложности 2/10.
Материал раунда
- Ветка:
issue/706-rereview-label, коммитae0e516af16e— ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. - Дерево материала:
2441daa60197945e66175706f34258bb589764c7git log --all --format='%H %T' | grep 2441daa60197 - Тело issue:
c4af86c1bc1383ec564be59c70334fb4158021c86e84c0d13f23951152fc619d - Вердикт конвейера:
green· High 0