houseplan-card/docs/reviews/CODE-REVIEW-730-r2.md

CODE-REVIEW #730 · заход r2

Материал: 3808e6f990694100bc687f09c3f25fdf59f2a91c (рабочая копия на этом SHA; git diff origin/dev...HEAD / git log --oneline origin/dev..HEAD). Трек: show. Дешёвые гейты (tsc --noEmit, npm test, npm run build со сверкой бандла) подтверждены зелёным Validate на этом SHA: https://github.com/Matysh/houseplan-card/actions/runs/36804205561 — не перегонялись. Ветка не приводилась к dev (трек show, #696): dev впереди на 20 коммитов, слияние без конфликта.

Скоуп

r1 разобрал AC1–AC3 целиком (два workflow-тела + страж ребейза разбирают отказ push кодом слияния вместо «любой отказ = сдвиг dev») и нашёл одну находку:

Medium (в скоупе AC2) — refusalSummary пишет «повтор и ребейз не помогут» для ЛЮБОГО не-stale исхода, в том числе workflow, где по PROCESS.md ребейз как раз штатное и единственное решение (делает его автор). Страж ребейза теперь (AC2) кладёт эту фразу в сводку шага — и вводит автора в заблуждение именно в момент, когда ему нужно понять, что делать.

r2 — точечный ответ на эту находку. Делта от материала r1 (981579aa) к материалу r2 (3808e6f9) — ровно:

 scripts/merge-candidate.mjs        |   7 ++-
 test/publish-push-refusal.test.mjs |   9 +++

(плюс коммит e0da2224, добавляющий сам документ r1 в дерево — технический, ревью не касается). Делта локальна: правка внутри одной функции (refusalSummary), новый тест рядом с уже существующими тестами той же функции. AC1–AC3 этой делтой не задеты — их материал и доказательства не менялись с r1.

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

1. Прочитан полный дифф ветки к origin/dev (443 строки) и делта r1→r2 (git diff 981579aa..3808e6f9) — отдельно, чтобы увидеть именно то, что изменилось в ответ на находку.
2. Прочитано тело issue #730 и все четыре комментария, включая финальный комментарий автора о r2 с указанием SHA 3808e6f9 и перечнем правок.
3. Прочитан новый код refusalSummary (scripts/merge-candidate.mjs:538-556): ветка nextStep для refusal.kind === PUSH_REFUSAL.workflow против общей фразы для остальных исходов.
4. Выполнена дисциплина «тест должен уметь падать»: временно подменил scripts/merge-candidate.mjs версией из r1 (981579aa) и прогнал test/publish-push-refusal.test.mjs — красным стал ровно и только новый тест #730 r1: при отказе по праву на workflow сводка зовёт автора сделать ребейз, а не отговаривает (1 fail из 24), остальные 23 прошли. Рабочая копия восстановлена (cp обратно сохранённой текущей версии), git status --short и git diff --stat после восстановления пусты.
5. На материале (3808e6f9) прогнаны все три задействованных теста-файла: test/publish-push-refusal.test.mjs — 24/24, test/merge-candidate.test.mjs — 33/33, test/rebase-generated.test.mjs — 19/19. Сумма 76/76 совпадает с заявлением автора в комментарии к r2.
6. Проверено (grep), что фраза «повтор и ребейз не помогут» осталась только как общий случай в scripts/merge-candidate.mjs, в тесте, который её же и фиксирует, и в архивном docs/reviews/CODE-REVIEW-730-r1.md — нигде больше в кодовой базе (PROCESS.md, комментарии к #705) несогласованного текста не осталось.
7. Сверено, что существующий комментарий к задаче при отказе по workflow (test/rebase-generated.test.mjs:435, из #705, не менялся) уже говорит «ребейз и push делает автор, либо владелец выдаёт право» — новый текст сводки (merge-candidate.mjs) теперь дословно согласован с этой формулировкой, а не придумывает новую.
8. Трейлеры коммита 3808e6f9: Issue: #730 ✓, User-Visible: no ✓ (правка текста CI-сводки, поведение карточки не меняется).
9. node scripts/smoke-select.mjs --base origin/dev --head HEAD — не перегонял повторно: src/** в делте r2 не тронут (как и во всей ветке), вывод r1 («исполняемого frontend-диффа нет») не мог измениться.

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

AC1–AC3 не входят в делту r2 — таблица из r1 остаётся в силе без изменений (см. «Унаследовано из r1»). Ниже — доказательство именно правки r2 (находка r1, в скоупе AC2):

Что доказывается	Доказательство	Чем краснеет (проверено исполнением)
При исходе workflow сводка больше не говорит «ребейз не поможет», а называет ребейз автора штатным выходом; при прочих отказах GitHub текст не изменился	test/publish-push-refusal.test.mjs — #730 r1: при отказе по праву на workflow сводка зовёт автора сделать ребейз, а не отговаривает	На refusalSummary из r1 (981579aa) — красный (проверено откатом файла и прогоном: 1/24 fail, именно этот тест)

Закрытие раунда r1

Находка r1	Чем закрыта	Где это видно
Medium (в скоупе AC2): refusalSummary пишет «повтор и ребейз не помогут» и для исхода workflow, хотя там ребейз — штатное решение автора	nextStep в refusalSummary теперь различает PUSH_REFUSAL.workflow и даёт отдельный текст: «повтор этого шага не поможет… ребейз и push делает автор, либо владелец выдаёт право»; для остальных исходов фраза не изменилась	scripts/merge-candidate.mjs:541-549 (новая ветка nextStep); воспроизведено исполнением — тест #730 r1 зелёный на 3808e6f9 и красный на 981579aa (проверено лично, см. «Как проверялось», п.4)

Унаследовано из r1

Принято без повторной проверки — делта r2 эти участки не касалась:

• AC1 (разбор push в _ship-review.yml/_beta-derived.yml через merge-candidate.mjs --push-refusal, устаревший lease — прежнее поведение, прочий отказ — стоп без повторов, причина и ответ git без токена в журнал и сводку) — документ docs/reviews/CODE-REVIEW-730-r1.md, материал 981579aa, раздел «AC · чем доказан · чем краснеет», строка AC1; перепроверено свежим прогоном: test/publish-push-refusal.test.mjs зелёный на 3808e6f9 (24/24, включая 12 тестов r1 плюс 1 новый).
• AC2 (страж ребейза пишет причину отказа в сводку шага) — тот же документ, строка AC2; перепроверено: test/rebase-generated.test.mjs зелёный на 3808e6f9 (19/19).
• AC3 (нет heredoc в run:, тонкие ship-review.yml/beta-derived.yml не тронуты) — тот же документ, строка AC3; делта r2 эти файлы не меняла (git diff 981579aa..3808e6f9 --stat их не перечисляет).
• Скрипт merge-candidate.mjs берётся из dev, не из тонкого вызывающего репозитория — документ r1, п.9 «Как проверялось»; код чекаута (ref: dev, git reset -q --hard origin/dev) делтой r2 не затронут.
• Токен нигде не светится (noisySecretsGone во всех новых тестах) — документ r1; новый тест #730 r1 тоже не вносит текст с токеном.
• Трейлеры коммитов r1 (981579aa) — документ r1, п.7 «Как проверялось».
• Полный npm test зелёный на материале r1 (3347/3348, 1 skip) — документ r1, п.5; на r2 полный прогон не повторялся (не требуется: делта — 2 файла, точечно прогнаны все три связанных тест-файла, 76/76, см. выше), наборы дешёвых гейтов подтверждены зелёным Validate на 3808e6f9.

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

• Находка r1 закрыта содержательно, а не косметически: текст для workflow-исхода называет единственно верное действие (ребейз и push автором либо передача права), текст для остальных исходов не поменялся — риск спутать «тупик» с «штатным шагом» снят для того единственного случая, где это было нужно.
• Новый текст согласован с уже существующим (немодифицированным, из #705) комментарием к задаче при том же исходе workflow — одна и та же мысль не разъезжается в двух разных местах процесса.
• Дисциплина «тест должен уметь падать» подтверждена лично (не на слово автора): откат merge-candidate.mjs к версии r1 красит ровно новый тест и ничего больше.
• Делта минимальна и не трогает AC1/AC3 — файлы _ship-review.yml, _beta-derived.yml, _process.yml в r2 не менялись, риска регрессии в уже проверенной r1 части нет.
• Трейлеры Issue: #730 / User-Visible: no на месте в коммите 3808e6f9; User-Visible: no оправдан — правка меняет только текст CI-сводки, не поведение карточки.

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

• tsc --noEmit, npm run build + сверка трёх копий бандла — не перегонял: подтверждены зелёным Validate на этом же SHA (3808e6f9, https://github.com/Matysh/houseplan-card/actions/runs/36804205561).
• Полный npm test (3348+ тестов) целиком на 3808e6f9 — не перегонял отдельно: делта r2 касается ровно трёх тест-файлов (publish-push-refusal, merge-candidate, rebase-generated), все три прогнаны поимённо и зелёные (76/76); остальные файлы делтой не затронуты, а Validate на этом SHA уже зелёный целиком.
• actionlint — делта r2 не трогает YAML workflow-тела (только scripts/merge-candidate.mjs и тест), не применимо.
• Браузерные смоки — src/** не тронут ни в r1, ни в r2; node scripts/smoke-select.mjs на материале r1 уже ответил «исполняемого frontend-диффа нет» (документ r1, «Чего не проверял»), делта r2 этот вывод не меняет.
• npm run golden:verify, python -m pytest tests_backend -q, npm run invariants -- --config …, performance-профили — не применимо по тем же причинам, что в r1 (нет меток/правок соответствующих областей).
• Ручной живой прогон _ship-review.yml/_beta-derived.yml/_process.yml в GitHub Actions — не делал; текст сводки проверен исполнением той же функции (refusalSummary), которую исполняет сам workflow-шаг.
• Мутанты по диффу — не запрашивались (трек show, #696), не прогонял.

Вердикт

Зелёный. Единственная находка r1 (Medium, в скоупе AC2) закрыта точечно и проверена исполнением: новый тест падает на коде r1 и проходит на коде r2. Делта r2 не задевает AC1/AC3, унаследованные без изменений. Блокирующих находок нет.

---

---

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

• Ветка: issue/730-derived-push-refusal, коммит 3808e6f99069 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: d1b6f2b4200c1166e905a9e9f87cec2be8bd102c

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

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