houseplan-card/docs/reviews/CODE-REVIEW-702-r1.md

CODE-REVIEW-702-r1

Issue: #702 · Трек: show · Заход: r1 · Блокирующих циклов израсходовано: 0 из 2 Материал ревью: 3a681e24e49038f202fc74cc3f5482e93e43977e (ветка issue/702-delete-merged-branches, один коммит поверх dev@c716bb0f)

Скоуп

Задача — гигиена веток конвейера (issue #702): 368 влитых issue/* веток висели на origin, список веток переставал что-то значить, агент, искавший ветку по номеру, мог взять устаревшую. Предложение из тела issue:

1. После успешного слияния merge-candidate.mjs удаляет ветку задачи; удаление — только если вершина ветки равна влитому кандидату.
2. Разовая чистка уже влитых веток — список владельцу, удаление только после явного согласия.
3. Не трогать codex/*, релизные и невлитые ветки.

Диапазон правки (git diff origin/dev...HEAD):

Файл	Класс	Что
scripts/merge-candidate.mjs	B (tooling)	deleteBranch() в realOps; вызов из finish() с branchTip, проброшенным из обеих веток слияния (fast-forward и push после Validate); текст комментария в commentFor()
PROCESS.md §10.4	C (docs)	пункт в списке шагов слияния
test/merge-candidate.test.mjs	B	4 новых теста (fakeOps получил deleteBranch/deleteOk)
scripts/mutation-registry.mjs	B	мутант merged-task-branch-kept

Файлов класса A нет — продуктовый код карточки не тронут, отсюда User-Visible: no в трейлере коммита (корректно). Пункт 2 (разовая чистка) в диффе не реализован кодом — по комментарию автора список подготовлен (branches-to-delete-702.txt), удаление не выполнялось, ждёт согласия владельца. Это соответствует AC: чистка — ручное действие владельца, не автоматизация этой задачи.

Ветка к dev не приводилась (трек show, #696): сливается с открытыми задачами #697–#700 без конфликта (git merge-tree, со слов автора; не перепроверялось — не относится к AC этой задачи).

Как проверялось

Ревью читало код и намеренно воспроизвело граничный случай, который юнит-тест подменяет фейком: сам механизм git push --force-with-lease на удаление ветки.

• Прочитан scripts/merge-candidate.mjs целиком: путь finish(), оба места, где branchTip попадает в extra (строка ~287 — fast-forward, ~332 — после зелёного Validate), и путь commentFor().
• Прочитан весь test/merge-candidate.test.mjs: fakeOps, все четыре новых теста и их согласованность с mergeCandidate().
• Выполнен независимый эксперимент на временном bare-репозитории (два прогона): git push --force-with-lease=refs/heads/X:<ожидание> origin :refs/heads/X — (а) при совпадении ожидания с реальной вершиной ветка удаляется (exit 0); (б) при устаревшем ожидании push отклоняется с ! [rejected] (delete) -> feature (stale info), ветка остаётся. Это подтверждает, что регэксп /stale info|rejected|fetch first|lease/i в deleteBranch() ловит именно ту ошибку, которую реально отдаёт git, а не придуманную автором строку.
• Прогнаны узкие автотесты и гейты, относящиеся к диффу (список — в таблице «Гейты» ниже).
• Проверены трейлеры коммита, сверка чисел (нет числа, видимого дважды — единственное число диффа, SHA/статусы, генерируется кодом, не дублируется руками).
• Проверено, что job integrate в _process.yml, вызывающая merge-candidate.mjs, использует не GITHUB_TOKEN (у которого в этом job только contents: read), а секрет HP_PROCESS_TOKEN через явный URL с токеном — то есть право на push/delete не зависит от permissions: блока job и не сужено этой задачей.
• Проверено, что ни один другой скрипт (task-packet.mjs, process-gate.mjs, process-metrics.mjs, wait-verdict.mjs) не предполагает существование ветки задачи после S8-merged — удаление ничего не ломает по цепочке.

AC · чем доказан · чем краснеет

AC	Чем доказан	Чем краснеет
Ветка удаляется после успешного слияния (fast-forward и push)	test/merge-candidate.test.mjs (2 теста), исполнено (26/26)	Мутант merged-task-branch-kept (if (false && merged && …)) — гард node --test --test-name-pattern="#702" test/merge-candidate.test.mjs; прогнан лично: node scripts/mutation-gate.mjs --id=merged-task-branch-kept → «merged-task-branch-kept: заявленный тест покраснел на мутанте», «поймано 1 из 1»
Удаление — только если вершина ветки на origin равна вершине, которую видело слияние (lease)	Юнит-тест «сдвинутая вершина — ветка остаётся» (deleteOk: false); плюс независимый ручной эксперимент на настоящем git (см. «Как проверялось») — подтверждает, что --force-with-lease на удаление действительно защищает от гонки, а не только в фейке	Ручной эксперимент — стерев --force-with-lease до простого git push origin :refs/heads/X, оба ручных прогона поменяли бы поведение (случай (б) удалил бы ветку вместо отказа); отдельного мутанта на этот случай нет, но реальный git-эксперимент закрывает то же самое доказательство подлинным исполнением, а не фейком
Красный/незавершённый merge не удаляет ветку (red Validate, conflict, reject-stale, rereview, give-up)	Юнит-тест: красный Validate → calls.filter(delete) == []; устаревшая ветка (#312) → calls.filter(delete) == []	Проверено чтением: во всех этих исходах finish() вызывается без branchTip в extra, а merged вычисляется из decision.action, который в этих ветках никогда не push/fast-forward — структурно недостижимо, что делает отдельный мутант избыточным для этой части
Разовая чистка 368 влитых веток — не автоматически, только после согласия владельца	Чтение диффа: код чистки в этом коммите отсутствует; issue-комментарий автора подтверждает, что ничего не удалено	Не мутант — это отсутствие функциональности, доказывается отсутствием кода, а не тестом

Находки

Нет High. Нет Medium в скоупе. Нет Medium вне скоупа.

Low (снята с записью, не блокирует): если deleteBranch() бросает исключение, НЕ совпадающее с lease-регэкспом (например, реальная ошибка прав или блокировка ветки правилом защиты) — оно ловится в finish() (catch (error) { ops.log(...) }), и branchDeleted остаётся null. Комментарий в issue в этом случае вообще не упоминает попытку удаления — ни «удалена», ни «оставлена: вершина сдвинулась». Различить в issue-комментарии «ветка осталась, потому что автор допушил коммит» (ожидаемо, задокументировано) от «ветка осталась из-за сбоя git/прав» (неожиданно) можно только по логу job в Actions, который не хранится вечно и не виден из issue. Не блокирует: инвариант «сбой удаления не отменяет слияние» соблюдён, риска для данных нет — в худшем случае лишняя ветка молча остаётся висеть, то есть today's статус-кво. Снимаю без правки: цена читаемости лога ниже цены нового условного пути в и так плотной функции finish().

Что проверено и корректно

• deleteBranch(ref, expected) в realOps использует тот же pushUrl (с токеном), что и pushWithLease, и тот же паттерн --force-with-lease=refs/heads/${ref}:${expected} — синтаксис реально поддерживается git для удаления (подтверждено экспериментом), не придуман.
• Оба места вызова finish(..., { branchTip }) передают корректную «последнюю вершину, которую видело слияние»: tip (= actual) в fast-forward-пути, где ветка задачи вообще не пушится, и candidate (уже реально запушенный в branch строкой раньше, pushWithLease(candidate, branch, tip)) в пути с ребейзом — совпадает с ожиданием юнит-тестов (cand-mat-on-dev1 и т. п.).
• Все исходы, где merged !== true (conflict, rereview, validation-red, validation-missing, give-up, reject-stale), структурно не передают branchTip и не вызывают удаление — проверено и чтением, и тестами.
• commentFor() добавляет к тексту «слито» одну из двух опциональных фраз строго по ctx.branchDeleted === true/false; при null — исходный текст без изменений, обратной совместимости с существующими тестами комментариев не сломано (assert.match(commentFor('push', ctx), /… слито$/) по-прежнему проходит, т.к. ctx.branchDeleted не задан → undefined).
• Мутант merged-task-branch-kept синтаксически корректен (find совпадает один-в-один со строкой файла), гард называет реальный тест, реестр цел (node --test test/mutation-gate.test.mjs — 69/69, структурные проверки реестра проходят).
• Трейлеры коммита: Issue: #702, User-Visible: no — верно, продуктовый код (класс A) не менялся.
• PROCESS.md §10.4 обновлён текстом, согласованным с кодом (обе ветки: «после push» покрывает и fast-forward, и обычный push).
• Ни один другой скрипт конвейера не читает ветку задачи после S8-merged — удаление ничего дальше по цепочке не ломает.
• Число, которое дифф делает видимым (368/370 влитых веток), — не дублируется в коде: это только текст комментария issue автора, не число в PROCESS.md или тесте; в самом диффе такого числа нет вовсе — нечему расходиться.

Чего не проверял

• npx tsc --noEmit, npm test (полный), npm run build + сверка трёх копий бандла — не перегонял: зелёный Validate на этом же SHA (3a681e24) уже подтверждён: https://github.com/Matysh/houseplan-card/actions/runs/36482861441
• Браузерные смоки — не прогонял: node scripts/smoke-select.mjs --base origin/dev --head HEAD сам ответил «исполняемого frontend-диффа нет (src/**/*.ts не тронут) — смоки этим диффом не выбираются, выбирать нечего». Явных смоков в AC issue не названо.
• npm run golden:verify — не прогонял, нет изменений рендера/UI.
• python -m pytest tests_backend -q — не прогонял, нет правок custom_components/**/*.py.
• npm run invariants — не прогонял, диффа геометрии/модели нет.
• Performance-профили — не названы в AC, не прогонял.
• Реальное поведение merge-candidate.mjs целиком на живом GitHub Actions (реальный gh/GH_TOKEN, реальные права HP_PROCESS_TOKEN на удаление ветки в этом конкретном репозитории) — не проверял; это уже ответственность реального прогона integrate при следующем слиянии, не предмет ревью кода. Логика вокруг него (аргументы, условия, регэксп ошибки) проверена чтением и одним независимым git-экспериментом на временном репозитории (см. «Как проверялось»).
• Разовая чистка 368 веток (пункт 2 issue) — не в этом коммите, поэтому не предмет код-ревью; когда владелец даст согласие и появится код/действие, это отдельный материал.

Гейты — сводка

Гейт	Прогнан	Результат
typecheck / npm test / build+bundle-policy	нет — Validate на SHA материала	зелёный: run 36482861441
node --test test/merge-candidate.test.mjs	да	26/26
node --test test/process-digests.test.mjs	да	5/5
node --test test/mutation-gate.test.mjs	да (целостность реестра)	69/69
node scripts/mutation-gate.mjs --id=merged-task-branch-kept	да	мутант убит, 1/1
node scripts/process-gate.mjs --range origin/dev..HEAD	да	0 предупреждений
node scripts/smoke-select.mjs --base origin/dev --head HEAD	да	нет frontend-диффа, смоки не выбираются
golden:verify	нет	не применимо (нет диффа рендера)
pytest tests_backend	нет	не применимо (нет правок Python)
invariants	нет	не применимо (нет правок геометрии)
ручной git-эксперимент force-with-lease на удаление ref	да	подтверждает регэксп ошибки в deleteBranch() соответствует реальному git

Вердикт

Зелёный. AC выполнены и доказаны (автотестом + мутантом + независимым ручным экспериментом с реальным git на защитную часть); находок, блокирующих или требующих правки в скоупе, нет; единственная Low-находка снята с записью.

---

Материал раунда

• Ветка: issue/702-delete-merged-branches, коммит 3a681e24e490 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: 387871be0d5bb95001506344a933150fb5ca5d16

  git log --all --format='%H %T' | grep 387871be0d5b
  

• Тело issue: 34ce91f776aff8956b77aea955e69a919ee4f52be89c8e94beb90f1e9beb5e85
• Вердикт конвейера: green · High 0