mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-03 13:18:58 +00:00
claude[bot]
@@ -0,0 +1,92 @@
|
||||
# CODE-REVIEW #730 · заход r4
|
||||
|
||||
**Материал**: `02dce77c9859ba26d855eb09e62090d126beecb8` (рабочая копия на этом 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/36817313433 (проверено лично через `gh run view 36817313433` — `headSha` совпадает, `conclusion: success`) — не перегонялись.
|
||||
|
||||
## Скоуп
|
||||
|
||||
r3 (`85d7c6fa`) получил зелёный вердикт — ребейз r2 на `dev`, ушедший на 1 коммит, чистый, без конфликтов. Зелёный вердикт бюджет §4 не тратит и цикла не образует (#227): «блокирующих циклов израсходовано 0 из 2» в заходе r4 корректно.
|
||||
|
||||
Между r3 и r4 `dev` продвинулся ещё на 20 коммитов, включая #726 и #727 (`scripts/ship-review.mjs`, переработка `_ship-review.yml`). Повторная публикация документа r3 снова уткнулась в отказ push по праву на workflow (тот же случай AC2), и страж ребейза опять вернул задачу автору на ребейз — автор выполнил его сам (комментарий 2026-10-01T04:54:07Z). На этот раз ребейз **не был чисто механическим**: автор сообщил о двух настоящих конфликтах — `test/publish-push-refusal.test.mjs` (тест `#726 AC5` и блок `#730` для `_ship-review.yml` независимы, оставлены оба) и `docs/reviews/INDEX.md` (пересобран `reviews-index.mjs`). Это не локальная делта в смысле §2.10 («ребейз на ушедший вперёд `dev`» — явно названный в инструкции повод для полного разбора), поэтому ниже разбираю весь дифф заново, а не только факт переноса SHA, и отдельно проверяю оба места конфликта по существу, а не на слово автора.
|
||||
|
||||
Дифф `origin/dev...HEAD` (файлы, без учёта архивных документов r1–r3): `.github/workflows/_beta-derived.yml` (17), `_process.yml` (5), `_ship-review.yml` (36), `PROCESS.md` (8), `scripts/merge-candidate.mjs` (12), `test/rebase-generated.test.mjs` (18) — построчно идентичны тому, что разбирали r1–r3 (см. «Как проверялось», п.2). Два файла дифф-стата изменились числом строк из-за реального слияния с `dev`: `docs/reviews/INDEX.md` (5→переcобран с учётом новых документов #726/#727/#732/#728) и `test/publish-push-refusal.test.mjs` (217 строк — включает унаследованный тест `#726 AC5`, который теперь физически соседствует с блоком `#730` в одном файле).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитано тело issue #730 и все одиннадцать комментариев, включая оба отказа push по праву на workflow (02:13 и 03:04), оба самостоятельных ребейза автора и финальный комментарий о втором ребейзе с перечнем конфликтов и прогона (`02dce77c` поверх `dev` `3b9f25ea`).
|
||||
2. Побайтово сверены неконфликтные файлы с содержимым, которое разбирали r1–r3: `git diff origin/dev...HEAD -- scripts/merge-candidate.mjs`, `.github/workflows/_process.yml`, `_ship-review.yml`, `_beta-derived.yml`, `PROCESS.md`, `test/rebase-generated.test.mjs` — хуки правки (новая ветка `nextStep` для `PUSH_REFUSAL.workflow`, ключи `ship-review`/`beta-derived`/`rebase` в `PUBLISHED`, `--summary="$GITHUB_STEP_SUMMARY"` в страже ребейза, разбор `push_err` в обоих телах публикации, убранный heredoc) текстуально совпадают с тем, что цитируют документы r1–r3. Расхождений нет.
|
||||
3. Прочитан конфликтный `test/publish-push-refusal.test.mjs` целиком вокруг места стыка (строки 1–40, 340–560): тест `#726 AC5` (строка 350) и блок `#730` (`// ---------- #730 _ship-review.yml ...` c строки 368) идут друг за другом без обрезанных строк, дублирования или следов конфликт-маркеров. Также найдено отличие от версии r1–r3: `STEP_SCRIPTS` теперь включает `'ship-review.mjs'` (было: `release-review.mjs, reviews-index.mjs, review-doc-guard.mjs, merge-candidate.mjs`). Проверено по существу, не предположением: `grep -n "ship-review.mjs" .github/workflows/_ship-review.yml` показывает, что именно шаг `Опубликовать документ` (тот же шаг, в который #730 добавляет разбор push-отказа) сам зовёт `scripts/ship-review.mjs` в нескольких местах (строки 307, 323, 384) — это код, добавленный в `_ship-review.yml` соседним #727, уже находящимся на `dev`. Добавление `ship-review.mjs` в `STEP_SCRIPTS` — необходимая адаптация замыкания импортов тестового песочницы под новый сосед по шагу, а не посторонняя правка: без неё песочница не нашла бы транзитивные импорты этого скрипта. Не находка.
|
||||
4. Весь дифф (`git diff origin/dev...HEAD`) и всё дерево (`git grep`) проверены на отсутствие следов незакрытого конфликта: `^<<<<<<<`, `^=======$`, `^>>>>>>>` — единственное совпадение внутри диффа оказалось строкой текста документа r3, описывающей саму эту проверку (цитата, не маркер); по дереву вне diff — пусто.
|
||||
5. `node scripts/reviews-index.mjs --dir=docs/reviews --check` → «docs/reviews/INDEX.md свеж» — пересборка индекса после переноса документов r1–r3 и соседних #726/#727/#728/#732 корректна, не полагаюсь на заявление автора «INDEX.md пересобран».
|
||||
6. Выполнена дисциплина «тест должен уметь падать» лично, на этом SHA, а не унаследована: временно откатил `nextStep` в `scripts/merge-candidate.mjs` к тексту до фикса r1/r2 (единая фраза «повтор и ребейз не помогут» для всех исходов) и прогнал `test/publish-push-refusal.test.mjs` → упал ровно и только тест `#730 r1: при отказе по праву на workflow сводка зовёт автора сделать ребейз, а не отговаривает` (24 pass / 1 fail из 25). Рабочая копия восстановлена из бэкапа; `git status --short` и `git diff --stat` после восстановления пусты.
|
||||
7. На материале (`02dce77c`) прогнаны все три связанных тест-файла: `node --test test/publish-push-refusal.test.mjs test/rebase-generated.test.mjs test/merge-candidate.test.mjs` → 77/77 pass, 0 fail (на один тест больше, чем в r2/r3 — это унаследованный из `dev` `#726 AC5`, не новый код этой задачи).
|
||||
8. `node scripts/smoke-select.mjs --base origin/dev --head HEAD` на этом SHA → «Исполняемого frontend-диффа нет», `src/**` по-прежнему не тронут (11 файлов всего, все инфраструктурные/документные).
|
||||
9. Трейлеры пяти коммитов делты r3→r4, которых не было в r3 (ребейз-результат и повторная публикация документа r3 в новой позиции плюс пересборка индекса): `8e42f4e9`/`56231394`/`4eb712be` несут `Issue: #730`, `User-Visible: no`; `0ab685e3`/`53bb3bf4` (публикация документа ревью r3, docs-only) и `02dce77c` (пересборка индекса) — тот же трейлер, формально не обязателен для docs-only коммитов, но не расходится и не вводит в заблуждение.
|
||||
10. Подтверждён зелёный Validate именно на этом SHA: `gh run view 36817313433` → `headSha: 02dce77c9859ba26d855eb09e62090d126beecb8`, `conclusion: success` — ссылка из инструкции совпадает с материалом ревью, не привязка «на слово».
|
||||
11. Признаков правки продуктового кода, PDF/geometry, `custom_components/**/*.py`, меток `ci:golden` нет — golden/pytest/invariants/perf вне применимости, как и в r1–r3.
|
||||
|
||||
## AC · чем доказан · чем краснеет
|
||||
|
||||
AC1–AC3 и закрытая в r2 находка не менялись контентно начиная с r2 — перепроверены исполнением на новом SHA, не только унаследованы, так как делта r3→r4 задела один из файлов-носителей доказательства (конфликт в `test/publish-push-refusal.test.mjs`):
|
||||
|
||||
| AC | Доказательство | Чем краснеет (проверено исполнением на `02dce77c`) |
|
||||
|---|---|---|
|
||||
| AC1 (разбор push, ship-review/beta-derived) | `test/publish-push-refusal.test.mjs` | 77/77 зелёных на материале; содержимое блока `#730` в файле после слияния с `#726 AC5` не повреждено (проверено чтением, п.3) |
|
||||
| AC2 (сводка стража ребейза, включая исправленный текст для `workflow`) | `test/rebase-generated.test.mjs` | входит в 77/77 выше; файл не конфликтовал при ребейзе, содержимое побайтово совпадает с r1–r3 (п.2) |
|
||||
| AC3 (нет heredoc, тонкие файлы не тронуты) | `git diff origin/dev...HEAD --stat` на тонких `ship-review.yml`/`beta-derived.yml` — пусто | статическая проверка, как в r1–r3; ребейз этого не касался |
|
||||
| Находка r1/закрытие r2 (текст для исхода `workflow`) | `test/publish-push-refusal.test.mjs:582-589` (`#730 r1`) | лично откатил `nextStep` в `merge-candidate.mjs` к досрочной версии → 1/25 fail ровно на этом тесте (п.6); восстановлено |
|
||||
|
||||
## Закрытие раунда r3
|
||||
|
||||
r3 — зелёный вердикт, возврата на правки не было (находок нет). Раздел неприменим по существу как «закрытие находки»; ниже — соответствие заявленному материалу r3 после второго ребейза.
|
||||
|
||||
| Что было заявлено в r3 | Чем подтверждено сейчас |
|
||||
|---|---|
|
||||
| Содержимое AC1–AC3 и находки r1/r2 не меняется ребейзом, только база | Подтверждено и для второго, уже неместного ребейза: неконфликтные файлы побайтово идентичны (п.2); конфликтные файлы прочитаны по существу и логика не искажена (п.3) |
|
||||
| 76/76 тестов в трёх связанных файлах | На новом SHA — 77/77 (+1 унаследованный из `dev` тест `#726 AC5`, не относящийся к #730) |
|
||||
|
||||
## Унаследовано из r1–r3
|
||||
|
||||
Принято без повторной проверки «с нуля» — содержимое логики не менялось с r1/r2, в этом раунде перепроверена именно та часть, которую затронул неместный ребейз (пп. 3, 6, 7 выше), а не всё целиком:
|
||||
|
||||
- Продуктовая корректность `refusalSummary`/`PUSH_REFUSAL.workflow`/`PUBLISHED` и формулировок для трёх стадий (`ship-review`, `beta-derived`, `rebase`) — документы r1 (находка), r2 (исправление), r3 (живое подтверждение в проде) — не переоценивал содержательно, т.к. `scripts/merge-candidate.mjs` в дифф-стате к `origin/dev` не изменился ни на строку со времён r2 (сверено, п.2); «тест умеет падать» перепроверил лично здесь же, а не положился на цитату r2/r3 (п.6).
|
||||
- Разбор push в `_ship-review.yml`/`_beta-derived.yml` настоящим bash/git во временных репозиториях (AC1) — документ r1, пп. 2–4; код шагов не менялся с r1 (п.2), перепроверено исполнением всего файла целиком (п.7).
|
||||
- Скрипт `merge-candidate.mjs` берётся из `dev`, не из тонкого вызывающего репозитория — документ r1, п.9; код чекаута не менялся.
|
||||
- Токен нигде не светится (`noisySecretsGone` во всех тестах) — документ r1; состав секретов в тестах не менялся, новый сосед (`#726 AC5`) — отдельный, независимый тест.
|
||||
- Полный `npm test` (3347+/3348, 1 skip) на материале, эквивалентном текущему по содержимому логики — документ r1, п.5; Validate зелёный целиком на самом этом SHA (п.10 выше), точечный прогон трёх файлов (77/77) заменяет повторный полный прогон, т.к. делта к r3 остальные ~3270 тестов не касается.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Второй ребейз, несмотря на реальные конфликты (в отличие от r2→r3), разрешён без потери содержания: оба конфликтных места (тест-файл и индекс документов) проверены по существу, а не по diffstat — структура тестов не повреждена, индекс пересобран инструментом и подтверждён как свежий самим генератором.
|
||||
- Единственное содержательное отличие теста от версии r1–r3 (`STEP_SCRIPTS` получил `'ship-review.mjs'`) объяснимо и корректно: вызвано тем, что сосед по той же строке workflow (#727) теперь сам зовёт этот скрипт внутри того же шага, который разбирает #730 — без этого расширения песочница теста не закрыла бы импорт.
|
||||
- Дисциплина «тест должен уметь падать» для находки r1/фикса r2 подтверждена лично на этом SHA (не унаследована бездоказательно): откат `nextStep` красит ровно один целевой тест.
|
||||
- Validate зелёный именно на материале ревью (`02dce77c`), сверено напрямую через `gh run view`, а не процитировано со слов.
|
||||
- Трейлеры (`Issue: #730`, `User-Visible: no`) на месте во всех новых коммитах делты; `User-Visible: no` оправдан — поведение карточки не меняется, изменение инфраструктурное.
|
||||
- Признаков незакрытого конфликта (маркеры, обрыв строк, дублирование блоков) нет нигде в дереве и в диффе.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- `tsc --noEmit`, `npm run build` + сверка трёх копий бандла, полный `npm test` (3348+ тестов целиком) — не перегонял: подтверждены зелёным Validate на этом же SHA (`02dce77c`, https://github.com/Matysh/houseplan-card/actions/runs/36817313433, сверено напрямую). Три связанных тест-файла (77 тестов) и дисциплину «тест умеет падать» перегнал лично, см. выше.
|
||||
- `actionlint` — бинарник недоступен в окружении ревью (`which actionlint` — пусто), как и в r1; полагаюсь на то же основание, что в r1: автор заявил «чистый» для затронутых тел, Validate проверяет синтаксическую валидность YAML тем же зелёным прогоном.
|
||||
- Браузерные смоки — `node scripts/smoke-select.mjs --base origin/dev --head HEAD` на `02dce77c` вернул «исполняемого frontend-диффа нет»; `src/**` не тронут ни в одном из четырёх раундов.
|
||||
- `npm run golden:verify`, `python -m pytest tests_backend -q`, `npm run invariants -- --config …`, performance-профили — не применимо: нет метки `ci:golden`, не менялся `custom_components/**/*.py`, не менялась геометрия, performance не названа в AC.
|
||||
- Ручной живой прогон `_ship-review.yml`/`_beta-derived.yml`/`_process.yml` в GitHub Actions с настоящим GitHub-отказом по workflow-праву — не ставил отдельно; сценарий `workflow` произошёл в реальности дважды между r2 и r4 (см. «Скоуп» r3 и этого документа) и оба раза повёл себя так, как требует AC2 — сильнее постановки вручную.
|
||||
- Мутанты по диффу — не запрашивались (трек show, #696), не прогонял.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. Материал r4 — второй ребейз на ушедший на 20 коммитов вперёд `dev` (включая #726/#727), на этот раз с двумя настоящими конфликтами в `test/publish-push-refusal.test.mjs` и `docs/reviews/INDEX.md`. Оба места разрешены корректно: тестовый файл не потерял и не исказил ни блок `#730`, ни соседний `#726 AC5`, добавленная строка `STEP_SCRIPTS` — обоснованная адаптация к новому соседству в том же workflow-шаге, индекс пересобран и подтверждён генератором как свежий. Логика продукта (`merge-candidate.mjs`, тела `_ship-review.yml`/`_beta-derived.yml`/`_process.yml`, `PROCESS.md`) не менялась со времён r2/r3 — перепроверена исполнением заново на этом SHA (77/77 тестов трёх связанных файлов, личный повтор дисциплины «тест умеет падать» для находки r1/фикса r2). Блокирующих находок нет.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/730-derived-push-refusal`, коммит `02dce77c9859` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `ab974f07f0b71626f99de337faf835539bd34091`
|
||||
```
|
||||
git log --all --format='%H %T' | grep ab974f07f0b7
|
||||
```
|
||||
- Тело issue: `3d77fb56ed2e308d5df6d329563f8143ebf6d19f6a36b320a92fdb76ee31d8fd`
|
||||
- Вердикт конвейера: `green` · High 0 · маршрут `fix`
|
||||
Reference in New Issue
Block a user