mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-03 05:08:53 +00:00
@@ -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; по прецеденту
|
||||
репозитория это отдельный последующий коммит публикации, не часть диапазона
|
||||
задачи.
|
||||
|
||||
## Находки
|
||||
|
||||
Нет.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/716-register-dispatch-workflows`, коммит `be0f95735118` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `993ff18663eee51f1c3312a5e3515d2e0a708852`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 993ff18663ee
|
||||
```
|
||||
- Тело issue: `9eab06f010a28bbbaccf0425202bd998c4f89b0361512585e9a82295f7deec14`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user