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

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

Материал: 981579aabbdbb93b4c514cad540735426e14b50e (ветка issue/730-derived-push-refusal, рабочая копия на этом 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/36790168783 — не перегонялись.

Скоуп

Два workflow-тела (_ship-review.yml — публикация SHIP-REVIEW в dev, _beta-derived.yml — бот-коммит производных артефактов беты) и страж ребейза в _process.yml считали/не полностью разбирали отказ git push:

• AC1. _ship-review.yml и _beta-derived.yml разбирают stderr push через merge-candidate.mjs --push-refusal=… --summary=…: устаревший lease — прежний повтор/совет перезапустить; отказ GitHub — шаг останавливается без повторов, причина и ответ git без токена — в журнал и в сводку шага. Доказательство — исполнение настоящим bash/git во временных репозиториях.
• AC2. Страж ребейза в _process.yml пишет причину отказа ещё и в сводку шага (--summary).
• AC3. Без многострочного текста/heredoc в run:; тонкие ship-review.yml/beta-derived.yml не меняются, правка — только в телах _*.yml.

Дифф: .github/workflows/_beta-derived.yml, .github/workflows/_process.yml, .github/workflows/_ship-review.yml, PROCESS.md, scripts/merge-candidate.mjs, test/publish-push-refusal.test.mjs, test/rebase-generated.test.mjs. Продуктовый код (src/**) не тронут — изменение инфраструктурное, картам SCOPE.md/USER-GUIDE соответствует как часть «процесс сам себя не ломает» (трек show, #696), видимого пользователю поведения карточки нет.

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

1. Прочитан дифф целиком (453 строки), PROCESS.md §11 (кусок про слияние/страж ребейза), issue #730 и оба комментария автора.
2. Прочитан scripts/merge-candidate.mjs — describePushRefusal, PUBLISHED, refusalSummary, pushRefusalMain (CLI --push-refusal/--summary/--comment).
3. Прочитаны изменённые куски _ship-review.yml, _beta-derived.yml, _process.yml построчно, сверены с текстом тестов (что именно шаг пишет в stdout/stderr/summary/dev).
4. Выполнено дисциплину «тест должен уметь падать»: временно откатил .github/workflows/_beta-derived.yml, _ship-review.yml, _process.yml и scripts/merge-candidate.mjs к origin/dev и прогнал новые тесты:
   • test/publish-push-refusal.test.mjs → 11 из 23 падают на старом коде (новые кейсы AC1/AC3 для ship-review и beta-derived).
   • test/rebase-generated.test.mjs → 2 из 19 падают на старом _process.yml (новые ассерты по сводке, AC2). Рабочая копия восстановлена на материал (git checkout HEAD -- …), git status --short и git diff --stat после восстановления пусты — рабочее дерево снова точно на материале.
5. На восстановленном материале прогнан полный npm test (3348 тестов, 3347 pass, 1 skip, 0 fail) и отдельно test/publish-push-refusal.test.mjs + test/rebase-generated.test.mjs (42/42 pass) — для перепроверки после отката/восстановления файлов.
6. node scripts/smoke-select.mjs --base origin/dev --head HEAD → «Исполняемого frontend-диффа нет», смоки не выбираются — браузерных смоков по диффу нет.
7. Сверены трейлеры коммита: Issue: #730 ✓, User-Visible: no ✓ (изменение не меняет поведение карточки, CHANGELOG/CHANGELOG.ru не тронуты — согласовано).
8. Сверено AC3: git diff origin/dev...HEAD --stat -- .github/workflows/ship-review.yml .github/workflows/beta-derived.yml — пусто, тонкие файлы не тронуты; grep -n "<<-?['\"]?[A-Za-z_]" по новым телам — heredoc не найден (использован только для test/publish-push-refusal.test.mjs, который и проверяет его отсутствие в телах).
9. Проверено, что scripts/merge-candidate.mjs в момент вызова берётся из dev (а не из тонкого вызывающего репозитория): actions/checkout с ref: dev в обоих job, плюс git reset -q --hard origin/dev перед каждой попыткой публикации ship-review — до разбора отказа.
10. Признаков правки продуктового кода, PDF/geometry, custom_components/**/*.py, меток ci:golden нет — golden/pytest/invariants/perf вне применимости (см. «Чего не проверял»).

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

AC	Доказательство	Чем краснеет (проверено исполнением)
AC1 (разбор push, ship-review/beta-derived)	test/publish-push-refusal.test.mjs, 12 новых тестов (успех/lease-повтор/отказ GitHub×3 для каждого из двух тел + AC3-тест)	11/23 тестов файла красные на origin/dev-версии _ship-review.yml/_beta-derived.yml/merge-candidate.mjs (проверено откатом и прогоном)
AC2 (сводка стража ребейза)	test/rebase-generated.test.mjs, доп. ассерты на r.summary/hook.summary в двух существующих тестах	2/19 тестов файла красные на origin/dev-версии _process.yml (проверено откатом и прогоном)
AC3 (нет heredoc, тонкие файлы не тронуты)	test/publish-push-refusal.test.mjs — тест «#730 AC3: …» (grep по телам, слайс run:, неразрезанность последней строки); дифф-stat на тонких файлах	проверено чтением + статическая проверка (grep/git diff --stat), не ad-hoc исполнением — тест-ассерт сам есть защита от регрессии

Находки

Medium (в скоупе AC2) — текст сводки «повтор и ребейз не помогут» вводит в заблуждение именно в сценарии, который страж ребейза обрабатывает как «не тупик»

scripts/merge-candidate.mjs:552 (refusalSummary) формирует для ЛЮБОГО не-stale исхода одну и ту же фразу:

«Это не сдвиг ${ref}: повтор и ребейз не помогут, шаг остановлен без повторов.»

Эта фраза теперь (с #730) пишется в сводку шага и при --stage=rebase, причём именно для исхода workflow. Но workflow — это единственный исход, где по тексту того же PROCESS.md (§11, строки рядом с правкой этой задачи) ребейз-таки помогает: «Отказ по праву на workflow … называется отдельно: ребейз и push делает автор, либо владелец выдаёт право». Страж ребейза это знает и обрабатывает отдельно: case workflow) → ::warning, exit 0, задача возвращается автору именно для того, чтобы тот сделал ребейз сам.

Воспроизведено (не только чтением — исполнением):

$ node -e "...classifyPushRefusal(...).kind === 'workflow'; refusalSummary(refusal, {ref:'issue/9-fix', stage:'rebase'})"
### git push в `issue/9-fix` отклонён: workflow (#723)

Ребейз ветки на dev не опубликован в `issue/9-fix`. GitHub отклонил push по
праву на workflow: у токена конвейера нет права создавать и менять
`.github/workflows/`. Это не сдвиг `issue/9-fix`: повтор и ребейз не помогут,
шаг остановлен без повторов.
...

Этот ровно текст и строку ($RUNNER_TEMP/summary.md) намертво фиксирует новый тест test/rebase-generated.test.mjs:427-429 (assert.match(r.summary, /…повтор и ребейз не помогут…/) — косвенно, через более узкий якорь, но сама фраза в файле присутствует и тестом не оспаривается).

Это ровно тот риск, который сам автор назвал в комментарии к задаче и оставил непочиненным: «Общий текст сводки «ребейз не поможет» для стража ребейза неточен: при отказе по праву на workflow автор может сделать ребейз сам.» — т.е. дефект самоопознан, но не исправлен, хотя находится в скоупе AC2 (это та самая сводка, которую AC2 просит писать). Для review-doc/release-review (#723) эта же фраза при исходе workflow менее значима — там отказ по праву на workflow для публикации документа ревью не является штатным, часто повторяющимся сценарием, а для rebase это именно штатная, документированная ветка.

Сценарий провала: кандидат меняет .github/workflows/*, страж ребейза получает ! [remote rejected] … refusing to allow a Personal Access Token to create or update workflow. Задача уходит в S6-in-progress, автор открывает сводку шага и читает «повтор и ребейз не помогут» — хотя корректное и единственное действие, которое ждёт от него процесс, это именно ребейз (своими правами) и push. Текст сбивает с толку в момент, когда автору и так нужно понять, что делать.

Почему Medium, не High: автоматика (код решения, метка, warning в логе отдельно от сводки) верна и не меняется этой находкой — ломается только пояснительный текст сводки, не поведение конвейера. Задача не блокируется функционально, но вводящий в заблуждение текст — это ровно то поведение, которое AC2 добавляет, и исправление тривиально (отдельная фраза для workflow-исхода, стадия rebase уже отличает его в case).

Это находка в скоупе задачи (AC2 как раз про текст сводки) → жёлтый вердикт, возврат автору, отдельный issue не заводится.

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

• AC1: _ship-review.yml и _beta-derived.yml действительно разбирают push-отказ через merge-candidate.mjs --push-refusal, stale ведёт себя как раньше (повтор/совет), остальные исходы останавливают шаг с причиной и ответом git без токена в журнале и сводке — проверено исполнением, тесты умеют падать.
• scripts/merge-candidate.mjs: добавление ключей ship-review/beta-derived/rebase в PUBLISHED корректно, refusalSummary и pushRefusalMain без побочных изменений в существующей логике для review-doc/release-review (дифф — только добавление записей и условного вызова appendFileSync, уже бывшего в #723 для других стадий).
• AC2: сводка стража ребейза пишется для исходов workflow/remote-rejected/unknown, не пишется для stale — соответствует требованию «у стража ребейза тоже, #730» и проверено исполнением (2 падающих теста на старом коде).
• AC3: heredoc в _ship-review.yml убран (сообщение коммита строится построчно в файл через { echo …; } > "$msg"), многострочного текста в run: нет; тонкие ship-review.yml/beta-derived.yml не изменены — проверено статически (git diff --stat, grep) и тестом файла.
• Скрипт merge-candidate.mjs, вызываемый из тел _ship-review.yml/_beta-derived.yml, берётся из dev (оба job чекаутят ref: dev, ship-review дополнительно сбрасывается на origin/dev перед каждой попыткой до вызова разбора) — не из тонкого вызывающего репозитория, что было бы несогласованно с остальной архитектурой (#705/#723).
• Токен не попадает ни в журнал, ни в сводку, ни в комментарий (noisySecretsGone проверяет во всех новых тестах, включая summary).
• Трейлеры коммита: Issue: #730 и User-Visible: no на месте; User-Visible: no оправдан — изменение не затрагивает видимое поведение карточки, changelog-файлы не тронуты.
• PROCESS.md обновлён точно и без лишнего — перечисляет новые стадии рядом с #723 и явно называет, что страж ребейза тоже пишет сводку (#730).
• Полный npm test на материале зелёный (3347/3348, 1 skip, 0 fail), после отката/восстановления файлов рабочая копия точно совпадает с материалом (git status --short, git diff --stat пусты).

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

• tsc --noEmit, npm run build + сверка трёх копий бандла — не перегонял: уже подтверждены зелёным Validate на этом же SHA (981579aa, https://github.com/Matysh/houseplan-card/actions/runs/36790168783). npm test я тем не менее перегнал сам (попутно с дисциплиной «тест должен уметь падать»), совпадает с Validate.
• actionlint — автор заявил «чистый», сам CLI в окружении ревью недоступен (не найден бинарник); полагаюсь на заявление автора плюс на то, что синтаксическая валидность YAML проверяется тем же Validate-прогоном, который уже зелёный на этом SHA.
• Браузерные смоки — node scripts/smoke-select.mjs --base origin/dev --head HEAD вернул «исполняемого frontend-диффа нет», смоки не выбираются; src/** не тронут. Решение: не прогонять — скрипт прямо сказал «нечего выбирать», это не НЕОПРЕДЕЛЁННОСТЬ.
• npm run golden:verify — метки ci:golden на issue нет, рендер не затронут, не прогонял.
• python -m pytest tests_backend -q — custom_components/**/*.py не менялся, не прогонял.
• npm run invariants -- --config … — геометрия и ссылки на неё не менялись, не прогонял.
• Performance-профили — не названы в AC, не прогонял.
• Ручная проверка живого запуска _ship-review.yml/_beta-derived.yml/_process.yml в GitHub Actions (реальный push, реальный GitHub-отказ) — не делал; разбор строится на исполнении тел настоящим bash/git во временных локальных репозиториях (как в test/publish-push-refusal.test.mjs/test/rebase-generated.test.mjs), это стандартный для этого набора тестов способ доказательства и ранее был достаточен для #705/#723.
• Мутанты по диффу — не запрашивались (трек show, #696), не прогонял.

Вердикт

Жёлтый. Единственная находка — Medium, в скоупе AC2, не блокирует автоматику, но вводит автора в заблуждение ровно в документированном сценарии «отказ по праву на workflow → ребейз делает автор сам». Правится в этой же ветке.

---

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

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

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

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