diff --git a/docs/reviews/CODE-REVIEW-730-r2.md b/docs/reviews/CODE-REVIEW-730-r2.md new file mode 100644 index 00000000..cfd91028 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-730-r2.md @@ -0,0 +1,95 @@ +# 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