diff --git a/docs/reviews/CODE-REVIEW-716-r2.md b/docs/reviews/CODE-REVIEW-716-r2.md new file mode 100644 index 00000000..8cb11b20 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-716-r2.md @@ -0,0 +1,222 @@ +# CODE-REVIEW — issue #716 · заход r2 + +Итог: High 0 · Medium 0 · Low 0 +Вердикт: зелёный + +## Материал раунда + +- Диапазон: `git log --oneline origin/dev..HEAD`, база `origin/dev` = `a5a73d15`. +- Коммиты (2): `5f505674` («ci: register ship-review and beta-derived as thin + callers in main (#716)») и `be0f9573` («docs: review document for #716», + публикация документа r1). +- Рабочая копия уже на `be0f957351181fa2ef72ae282e439a2917636ca2` — `HEAD detached`, + совпадает с меткой материала из системного промпта. `git fetch`/`checkout` на + другой коммит не делал; единственный вызов `git fetch origin dev` не менял + `origin/dev` (уже был на `a5a73d15`) и не переключал рабочую копию. +- Причина r2 — не новая правка, а ребейз: владелец подтвердил в issue + «Изменение задачи то же, что в зелёном r1» (комментарий `20:49:31Z`). + Ребейз на ушедший вперёд `dev` прямо назван в промпте как случай, не + сокращающий объём разбора («разбор остаётся ПОЛНЫМ»), поэтому ниже — + полная проверка AC1–AC3 на текущем SHA, а не только сверка дельты. +- Validate на `be0f9573`: success, + https://github.com/Matysh/houseplan-card/actions/runs/36775411384 — я лично + проверил `gh run view 36775411384 --json headSha,conclusion`: `headSha` == + `be0f957351181fa2ef72ae282e439a2917636ca2`, `conclusion` == `success`. + Принят по канону (§4), `tsc`/`test`/`build` не перегонял. + +## Скоуп + +Задача чинит процессный дефект инфраструктуры: `ship-review.yml` (#696) и +`beta-derived.yml` (#697) существовали только в `dev`. `workflow_dispatch` +исполняет файл с выбранной ветки, но GitHub регистрирует workflow и разрешает +сам запуск (кнопка, `gh workflow run`, API) только для файла, который есть в +ветке по умолчанию (`main`) — без этого обе беты не запускались вовсе. +Решение — устройство #623 (тонкий вызывающий в `main`, тело `_<имя>.yml` по +`@dev`), распространённое на эти два файла. + +Меняются только `.github/workflows/*`, `PROCESS.md`, +`scripts/mutation-registry.mjs`, тесты и (в составе диапазона) документ ревью +r1 — продукта нет. Трек `show`, инфраструктура; `docs/SCOPE.md` не +затрагивается (нет пользовательского поведения, привязки к Core user jobs не +требуется — прецедент тот же, что у #623/#704/#714 в этом же репозитории). +`User-Visible: no` в трейлере `5f505674` соответствует диффу: ни одного файла +из `custom_components/**`, `src/**`, обоих changelog. + +## Как проверялось + +1. **Диапазон и идентичность содержимого.** `git diff origin/dev...HEAD --stat` + — 11 файлов, `739 insertions(+), 498 deletions(-)`. Прочитал полный + `git diff` по всем изменённым файлам (не только диффстат): новые тела + `_ship-review.yml` (333 строки) и `_beta-derived.yml` (215 строк) — + построчная копия прежних монолитных job'ов с заменой шапки на + `workflow_call`; вызывающие `ship-review.yml`/`beta-derived.yml` в `dev` + стали тонкими (`on: workflow_dispatch`, права, concurrency, одна job + `uses: …/_<имя>.yml@dev` c `secrets: inherit`) — устройство #623 + применено без отклонений. +2. **AC1 (регистрация).** + - `gh api repos/Matysh/houseplan-card/actions/workflows` — лично прогнал + сейчас: оба workflow перечислены, `state: "active"` для обоих (`Бета: + производные артефакты на dev`, `Бета: пакетное ревью ship`) — прямое + доказательство, что регистрация в силе, а не только состоялась в + прошлом. + - Зеркало в `main`: `git log --oneline -3 origin/main` показывает + `2022e183 ci: mirror thin workflow callers in main after #716` перед + `7d4d75bd` (v1.78.0). Сравнил побайтово: + `diff <(git show origin/main:.github/workflows/ship-review.yml) + <(git show HEAD:.github/workflows/ship-review.yml)` и то же для + `beta-derived.yml` — пусто по обоим. Значит зеркало в `main` и текущий + кандидат на `dev` идентичны; порядок публикации (сначала `main`, потом + `dev`) выполнен на практике, а не только продекларирован. + - Структура тонкий/тело проверена и чтением, и тестом (см. AC2): единая + job, `uses: …/_<имя>.yml@dev`, `secrets: inherit`, каждый вход + `workflow_dispatch` проброшен во `workflow_call`, потолок прав — + объединение прав job тела. +3. **AC2 (сверка).** + - `validate.yml`/`workflow_sync`: цикл сверки расширен до восьми файлов + (`+ ship-review.yml beta-derived.yml`), прочитал diff — соответствует. + - `test/default-branch-workflows.test.mjs`: новый набор + `DISPATCH_BEFORE_PROMOTION`, новый тест `#716: шаги беты по кнопке — + тонкие файлы наравне с исполняемыми из main` проверяет и список из + восьми имён, и то, что каждый файл в списке существует и его + единственный триггер — `workflow_dispatch` (иначе запись избыточна). + - **Мутанты воспроизведены лично** (трек `show` этого не требует — + REVIEWER.md прямо говорит «мутанты в разработке не гоняются ни на каком + треке — ревьюер их тоже не применяет»; я прогнал их сверх минимума для + собственной уверенности, а не по обязанности): + - мутант `validate.yml` (убрать `ship-review.yml beta-derived.yml` из + цикла сверки, `scripts/mutation-registry.mjs:10838`) — применил patch, + `node --test test/default-branch-workflows.test.mjs` → + **1 из 45 упал** (тест `#716: шаги беты по кнопке — тонкие файлы + наравне с исполняемыми из main`), откатил, `git status --short` пусто. + - мутант `_beta-derived.yml` (убрать `echo "Release: $TAG"`, + `scripts/mutation-registry.mjs:13798`, путь уже перенацелен с + `beta-derived.yml` на `_beta-derived.yml`) — применил, `node --test + test/beta-derived.test.mjs` → **1 из 4 упал** (провенанс-тест), + откатил, рабочая копия чистая. + - `node scripts/mutation-registry-check.mjs` → exit 0 — реестр + непротиворечив, переименование пути мутанта не оставило «мёртвого» + `find`. +4. **AC3 (документация).** Прочитал diff `PROCESS.md`: §10.4 перечисляет + восемь файлов с разбивкой «по событию» / «по кнопке, но до промоушена», + объясняет, почему `workflow_dispatch` не даёт запуск без файла в `main`, и + задаёт порядок публикации («сначала `main`, затем `dev`»). §8 и §11.7 + получили точную команду запуска (`gh workflow run … --ref dev -f tag=…` + или кнопка) с отсылкой к тонкому/телу. Прежний неверный комментарий («файл + исполняется с ветки прогона, зеркало не нужно») убран из обоих файлов и + заменён корректным объяснением. `grep -rn "шест[ьи] файл" PROCESS.md + AGENTS.md docs/process/*.md` — ничего не найдено, старых упоминаний «шести + файлов» не осталось. +5. **Таблица для защитного AC (AC2 — гард сверки).** + + | AC | чем доказан | чем краснеет | + |---|---|---| + | AC2 (`workflow_sync` держит main/dev в синхроне для новых 2 файлов) | `test/default-branch-workflows.test.mjs`, тест `#716: …`; лично прогнанный мутант `validate.yml` (убрать 2 файла из цикла сверки) | 1/45 тестов красный (см. пункт 3 выше) | + +6. **Тесты.** `node --test test/default-branch-workflows.test.mjs + test/beta-derived.test.mjs test/ship-review.test.mjs` → 57/57 green. + Расширил до полного набора workflow-тестов, которые называл автор: + `test/default-branch-workflows.test.mjs test/validate-workflow.test.mjs + test/beta-derived.test.mjs test/ship-review.test.mjs + test/process-workflow*.test.mjs` → **85/85 green** — совпадает с тем, что + автор заявил после ребейза. +7. **Побочные гейты.** `node scripts/action-pins.mjs` → «все сторонние + Actions закреплены полным SHA» (новых `uses:` нет — пины из прежних + монолитных файлов перенесены без изменений). `node scripts/smoke-select.mjs + --base origin/dev --head HEAD` → «Исполняемого frontend-диффа нет… + Browser-smoke этим диффом не выбираются… Тронуто файлов: 11.» — браузерные + смоки к этому диффу не относятся, вход и решение приложены. +8. **Трейлеры.** `git show -s --format=%B` для `5f505674`: `Issue: #716`, + `User-Visible: no` — соответствуют диффу (изменений пользовательского + поведения нет, оба changelog не тронуты и не должны быть). Коммит + `be0f9573` (документ ревью r1) несёт те же трейлеры корректно. +9. **INDEX.md.** `docs/reviews/INDEX.md` пока не знает про #716 — это + ожидаемо: по прецеденту `#704`/`#714` в этом репозитории индекс обновляет + отдельный последующий коммит бота («docs(reviews): индекс после сдвига + каталога (#NN)»), а не коммит с самим документом ревью. Не находка. + +## Закрытие раунда r1 + +Раунд r1 был вынесен зелёным на коммите `5451542e`, который затем стал +недостижим: во время ревью `dev` ушёл на 11 коммитов вперёд, слияние +отменил страж (#312), задачу вернули в `S6-in-progress`, и владелец +перебазировал ветку (`d41cca76` → `be0f9573`, force-with-lease). Открытых +находок у r1 не было (High 0 · Medium 0), поэтому таблицы «находка | чем +закрыта» не требуется по существу — ниже фиксирую, что сам ребейз не внёс +расхождений с тем, что r1 проверил: + +| Что проверял r1 на `5451542e` | Что я перепроверил на `be0f9573` | Результат | +|---|---|---| +| `GET /actions/workflows` — оба workflow `active` | Тот же вызов сейчас | Совпадает, всё ещё `active` | +| Зеркало в `main` (`2022e183`) побайтово равно кандидату | `diff` зеркала `main` против текущего `HEAD` по обоим файлам | Совпадает, пусто | +| 45/45 `default-branch-workflows.test.mjs`, оба мутанта лично воспроизведены | Те же 2 мутанта лично воспроизведены на `be0f9573`, плюс полный набор 85/85 | Совпадает и расширено | +| §10.4/§8/§11.7 называют 8 файлов и порядок публикации | Тот же текст (diff `PROCESS.md` идентичен по содержанию) | Совпадает | + +Различие между `5451542e` и `5f505674` — только перестановка коммита поверх +нового `dev` (ребейз без конфликтов, подтверждено автором и видно по +идентичности диффа `origin/dev...HEAD` содержанию, описанному в r1). Новых +файлов, новых AC или изменённого поведения ребейз не внёс. + +## Унаследовано из r1 + +- Полный вывод `GET /actions/workflows` (регистрация обоих workflow) — + переподтверждено заново в этом раунде (не унаследовано вслепую, см. выше). +- Оценка, что задача не расширяет скоуп за пределы трёх AC и не имеет + побочных продуктовых изменений — принято повторно после перечтения + полного диффа на `be0f9573`, расхождений с оценкой r1 не найдено. +- Ничего не принято по одному лишь упоминанию в документе r1 без + собственной проверки: каждый пункт «Как проверялось» выше — это мой + собственный прогон/чтение на текущем SHA. + +## Что проверено и корректно + +- AC1: оба workflow — `active` в `GET /actions/workflows` прямо сейчас; + зеркало в `main` побайтово равно кандидату; тонкий/тело — по автоматическому + структурному тесту и по чтению. +- AC2: восемь файлов сверяются `workflow_sync` и структурным тестом; оба + мутанта лично воспроизведены и красят соответствующие тесты; реестр + мутаций внутренне непротиворечив. +- AC3: §10.4 называет восемь файлов, причину и порядок публикации; §8 и + §11.7 дают точную команду запуска; неверный комментарий убран из обоих + файлов. +- Коммиты несут корректные `Issue:`/`User-Visible:` трейлеры, соответствующие + диффу. +- Ребейз не изменил содержание изменения — подтверждено прямым диффом и + повторной проверкой живых фактов (API, байтовое сравнение зеркала), а не + только заявлением автора. + +## Чего не проверял + +- Полный `npx tsc --noEmit` / `npm test` / `npm run build` со сверкой трёх + копий бандла — не перегонял: Validate зелёный на этом самом SHA + (`be0f9573`, ссылка выше), принято по канону §4. +- Реальный dispatch `ship-review.yml`/`beta-derived.yml` (кнопкой или + `gh workflow run`) — не запускал: дорого, не полностью обратимо (коммитит + в `dev`, запускает настоящего агента ревью), и автор сам отложил это до + после слияния. +- `npm run golden:verify` и `python -m pytest tests_backend` — не требуются: + нет метки `ci:golden`, `custom_components/**/*.py` не тронут. +- `npm run invariants` — не требуется: геометрия не тронута. +- Ограничение токена сессий (`Actions: write` для `publish-prerelease.yml`) — + явно вынесено автором за рамки задачи в теле issue и не входит ни в один + AC. +- `docs/reviews/INDEX.md` для #716 — не обновлён на этом SHA; по прецеденту + репозитория это отдельный последующий коммит публикации, не часть диапазона + задачи. + +## Находки + +Нет. + +--- + + + +## Материал раунда + +- Ветка: `issue/716-register-dispatch-workflows`, коммит `be0f95735118` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `993ff18663eee51f1c3312a5e3515d2e0a708852` + ``` + git log --all --format='%H %T' | grep 993ff18663ee + ``` +- Тело issue: `9eab06f010a28bbbaccf0425202bd998c4f89b0361512585e9a82295f7deec14` +- Вердикт конвейера: `green` · High 0