Files
2026-09-29 05:04:38 +00:00

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 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: 2441daa60197945e66175706f34258bb589764c7
    git log --all --format='%H %T' | grep 2441daa60197
    
  • Тело issue: c4af86c1bc1383ec564be59c70334fb4158021c86e84c0d13f23951152fc619d
  • Вердикт конвейера: green · High 0