diff --git a/docs/reviews/CODE-REVIEW-514-r4.md b/docs/reviews/CODE-REVIEW-514-r4.md new file mode 100644 index 00000000..ec5e3a2f --- /dev/null +++ b/docs/reviews/CODE-REVIEW-514-r4.md @@ -0,0 +1,170 @@ +# CODE-REVIEW-514-r4 + +- **Issue:** https://github.com/Matysh/houseplan-card/issues/514 — «E2E на реальном HA как гейт стабильного релиза» +- **ТЗ:** `docs/specs/514-e2e-stable-release-gate.md` (зелёное ревью ТЗ, `docs/reviews/SPEC-REVIEW-514-r1.md`, r1, High 0/Medium 0) +- **Материал ревью:** ровно `0ce5e4eed3d1d2e3a52f07c35dbc9a8297dd01a2` (рабочая копия на нём). `git log --oneline origin/dev..HEAD` — 9 коммитов. `git diff origin/dev...HEAD` — 15 файлов, +1033/−3. +- **Заход:** r4 · блокирующих циклов израсходовано 1 из 4 (израсходован жёлтым вердиктом r3; зелёный вердикт бюджет не тратит, #227) + +## Материал предыдущего раунда (SHA не резолвится — обычное дело, §2.10) + +r3 (`docs/reviews/CODE-REVIEW-514-r3.md`, вердикт жёлтый, комментарий issue 2026-09-09T21:16:28Z) объявил материал `d166bad3eb293d70cdd37cc715c639449742d988`. Этот объект в текущем репозитории уже не резолвится: + +``` +git cat-file -t d166bad3eb293d70cdd37cc715c639449742d988 +→ fatal: git cat-file: could not get object info +``` + +Между r3 и этим заходом случился ещё один сдвиг базы (см. комментарий issue 2026-09-09T21:21:59Z: `workflow_sync` увидел, что `main` отстал от `dev` по `process.yml` после слияния #515, конвейер сделал зеркальный коммит `aeb0a473` и пере-опубликовал ветку задачи) — commit-хэши коммитов задачи пересчитались от нового родителя, содержимое не изменилось. Якоря по дереву/блобу, которые пережили бы это (`git log --all --format='%H %T' | grep <дерево>`), r3 не оставил — его собственный ручной блок «Материал раунда» назвал только commit SHA, а машинный блок конвейера в конце документа относится к более раннему, никогда не рецензированному состоянию (`6d28e170`, до второго ребейза). Восстановление сделано по содержимому, не по имени: + +``` +git diff origin/dev...a8a9b7d9 --stat +→ 14 files changed, 824 insertions(+), 3 deletions(-) + (тот же список файлов и те же числа, что в шапке CODE-REVIEW-514-r3.md) +git merge-base --is-ancestor a8a9b7d9 HEAD && echo yes → yes +git log --oneline a8a9b7d9..HEAD +→ 0ce5e4ee ci: the E2E gate fails loudly when the release list cannot be read + ed6d1d76 docs: review document for #514 +``` + +`a8a9b7d9` — content-эквивалент `d166bad3` (тот же диапазон правок, то же число файлов/строк, тот же список путей), переживший последующий ребейз под другим именем. Это и есть материал, на котором был вынесен вердикт r3; от него считается дельта этого раунда. + +**Почему разбор по дельте, а не полный.** База `origin/dev` не продвинулась между r3 и r4 (`git merge-base origin/dev HEAD` и `git merge-base origin/dev a8a9b7d9` — оба `ad2858a8`, один и тот же коммит): ребейза на ушедший вперёд `dev` в этот раз не было, только пересборка ветки поверх той же базы (мираж-коммит `aeb0a473` ушёл в `main`, не в диапазон задачи). Контракт поведения не менялся, новая подсистема не задета, объём дельты (2 коммита, 3 файла, +24/−5 строк) на порядок меньше исходной задачи. Условие «разбор остаётся полным» (§2.9/§2.10) не выполнено ни по одному пункту — сокращённый разбор оправдан. + +## Дельта r3 → r4 + +``` +git diff a8a9b7d9..HEAD --stat +``` + +| Коммит | Класс | Содержимое | +|---|---|---| +| `ed6d1d76` | C (docs) | публикация `docs/reviews/CODE-REVIEW-514-r3.md` конвейером — только новый файл, продуктовых строк нет | +| `0ce5e4ee` | B+C | `scripts/e2e-gate.mjs` (+11/−2), `test/e2e-gate.test.mjs` (+12), `PROCESS.md` (+3/−3) — фикс M1(r3) и формулировки L4(r3) | + +Класс A не задет (как и во всём диапазоне `origin/dev..HEAD`); трейлеры `Issue: #514` / `User-Visible: no` на обоих коммитах корректны (`git show -s --format=full`). + +## Закрытие раунда r3 + +| Находка r3 | Чем закрыта | Где это видно | +|---|---|---| +| **M1** (Medium, в скоупе) — фолбэк-токен, scoped только на `houseplan-e2e`, молча ломает `previousStable` через проглоченную ошибку `gh release list` в `releases()`, воспроизводя дефект `4143f998` под чужим диагнозом | `scripts/e2e-gate.mjs`: `releases()` теперь проверяет `r.status !== 0` и бросает `Error` с текстом `gh release list : ` — так же, как уже делал `dispatch()`; исключение попадает в тот же `catch` блок `e2eGate` → `result: 'error'` с `TOKEN_HINT`. `TOKEN_HINT` переписан: называет оба репозитория и обе нужные операции («Actions: write на houseplan-e2e И чтение релизов houseplan-card»), а не только первый | `scripts/e2e-gate.mjs:28,80,119-123`; новый тест `test/e2e-gate.test.mjs:117-127` (`#514 r3 M1`) — см. проверку ниже | +| **L4** (Low, новая в r3) — `PROCESS.md:704-706` путал подлежащее и дополнение («E2E … запускает и ждёт `release.yml`», хотя наоборот) | Строка переписана: «`release.yml` сам запускает `e2e.yml` в `houseplan-e2e` на теге и ждёт его зелёного (#514)» | `PROCESS.md:704-706` (diff выше) | +| L1 (унаследована из r1/r2, подтверждена чтением в r3) | Не требовала правки — Low принят без изменений | код `classifyRun`/цикл ожидания в `e2eGate` (строки ~62-97) не тронут дельтой r3→r4, см. «Унаследовано» | +| L2 (унаследована из r1/r2) | Не требовала правки — файл вне материала процесса houseplan-card | не тронут | +| L3 (унаследована из r1/r2) | Не требовала правки — принятый паттерн из `validate-gate.mjs` | не тронут | + +**Мутация подтверждает закрытие M1, а не заявление автора.** Временно откатил фикс (вернул `releases()` к варианту «проглотить ошибку», без `throw`) и прогнал новый тест: + +``` +node --test test/e2e-gate.test.mjs (с откаченным releases()) +→ not ok 11 - #514 r3 M1: ... + error: 'Missing expected rejection.' +``` + +Тест краснеет на снятой защите — «умеет падать» подтверждено, не заявлено. Восстановил `scripts/e2e-gate.mjs` из бэкапа, `git status --short` — пусто, рабочая копия чиста. + +С фиксом на месте: + +``` +node --test test/e2e-gate.test.mjs test/release-workflow.test.mjs +→ # tests 15 # pass 15 # fail 0 (было 14/14 на материале r3 — новый тест M1 прибавил один) +``` + +| AC / защита | Чем доказан | Чем краснеет | +|---|---|---| +| AC1, ветка отказа `releases()` (M1) | `test/e2e-gate.test.mjs:117-127`, тест `#514 r3 M1` | откат `releases()` к варианту без `throw` → `not ok`, `Missing expected rejection` (прогнано лично, см. выше) | + +Мутанты `scripts/mutation-gate.mjs` дельту не затронули (файл не менялся между r3 и r4) — прогнаны все три для очистки: + +``` +node scripts/mutation-gate.mjs --id=release-ships-on-red-e2e → поймано 1 из 1 +node scripts/mutation-gate.mjs --id=release-upgrades-stable-onto-itself → поймано 1 из 1 +node scripts/mutation-gate.mjs --id=release-trusts-foreign-e2e-run → поймано 1 из 1 +``` + +## Унаследовано из r3 + +Без повторной проверки в этом раунде принято (делта их не задевает): + +- **AC2** (опознание своего прогона по `HP `) и **AC4** (`journeys-dev` только по расписанию) — код `classifyRun`/`isOurRun` (`scripts/e2e-gate.mjs:62-97`) и контракт `houseplan-e2e/e2e.yml` дельтой не тронуты; проверены в r3 напрямую (`gh api repos/Matysh/houseplan-e2e/contents/...`), см. `docs/reviews/CODE-REVIEW-514-r3.md`, материал (content-эквивалент) `a8a9b7d9`. +- **AC3** (пре-релизы шаг не выполняют) — `.github/workflows/release.yml` не входит в дельту r3→r4 (`git diff a8a9b7d9..HEAD -- .github/workflows/release.yml` пуст); `test/release-workflow.test.mjs` не менялся и зелёный (см. прогон выше, 2/15 из общего числа). +- **AC5** (три мутанта ловят снятую защиту) — код мутантов не менялся; перепрогнан в этом раунде для очистки (см. таблицу выше), не унаследован голословно. +- **L1** (плановая job `houseplan-e2e/plan` упадёт раньше первой именованной суб-job → `missing`) — строки `scripts/e2e-gate.mjs:62-97`, где живёт этот путь, не входят в дельту `a8a9b7d9..HEAD`. Инвариант «не публиковать на красном» не нарушается. +- **L2** (README `houseplan-e2e` может буквально описывать `upgrade_from=stable`) — внешний файл, вне диапазона `origin/dev...HEAD` в обоих раундах. +- **L3** (окно `gh run list --limit 10`) — код опроса не менялся. +- Порядок `gh release list` (новые первыми), на котором стоит `previousStable` — код `previousStable` не в дельте r3→r4, подтверждён в r1/r2/r3 (см. цепочку в `CODE-REVIEW-514-r3.md`, раздел «Унаследовано из r2»). +- Документация вне `PROCESS.md:704-706` (`AGENTS.md`, `docs/DEVELOPMENT.md`, `docs/TESTING.md`, `docs/specs/README.md`) — не в дельте, была сверена построчно в r3. + +## Дешёвые гейты — уже подтверждены на этом SHA + +Validate на `0ce5e4ee` зелёный: https://github.com/Matysh/houseplan-card/actions/runs/34406969841 (в тексте раунда это дано как факт; проверил `head_sha` не переспрашивал повторно — ссылка та же, что в условии раунда). Значит `npx tsc --noEmit`, `npm test` (полный), `npm run build` со сверкой бандла не перегонялись — правило сужения гейтов прямо это разрешает. Дополнительно лично прогнано (диф это код меняет): + +- `node --test test/e2e-gate.test.mjs test/release-workflow.test.mjs` → 15/15. +- Мутация на M1 (откат защиты → тест краснеет) — см. выше. +- Три именованных мутанта `mutation-gate.mjs` → 1/1 каждый. +- `node scripts/process-gate.mjs --issues` → «гейт пройден, предупреждений 1» (п.8, ожидаемо — класс A не задет). + +**Не прогонялось и почему:** `golden:verify` (дельта не меняет рендер), `check-docs.mjs`/`model-invariants.mjs`/браузерные смоки (дельта не трогает `src/**`, геометрию или `layout`), `pytest tests_backend` (Python не тронут), performance-профили (не названы в AC, перф-путь не тронут). Полный список `demo/smoke_*.mjs`/`smoke-select.mjs` не запрашивал — выбирать нечего, `src/**` не в диапазоне ни разу за все 4 раунда. + +**Одно число — один источник.** Неприменимо: дельта не добавляет и не меняет ни одной пользователем видимой величины (изменения — в тексте ошибки CI-скрипта и в формулировке `PROCESS.md`). + +## Новая находка + +### L5 (Low, новая в r4) — `docs/specs/514-e2e-stable-release-gate.md:38` дословно цитирует старый текст `TOKEN_HINT`, который правка этого раунда изменила + +Спецификация иллюстрирует ошибку отказа токена цитатой: «текстом «нужен секрет `E2E_DISPATCH_TOKEN` с правом Actions: write на houseplan-e2e»». После фикса M1 фактический `TOKEN_HINT` (`scripts/e2e-gate.mjs:28`) — «нужен секрет `E2E_DISPATCH_TOKEN`: Actions: write на houseplan-e2e И чтение релизов houseplan-card (fine-grained PAT — оба репозитория в списке)». Цитата в ТЗ устарела относительно кода. + +Не блокирует: ТЗ — иллюстративный, не нормативный текст в этом месте (описывает пример сообщения, а не контракт с проверяемым AC на точную строку), спецификации не переоткрываются задним числом при каждой правке реализации (§2.3 — статус ТЗ не меняется после `S5-ready`), и ни один AC/тест не сверяет спек-текст с `TOKEN_HINT` дословно. Оставляю Low без правки, с записью: если задача вернётся на ещё один правочный цикл по другой причине, эту строку стоит поправить заодно. + +## Что проверено и корректно + +- M1(r3) закрыт по существу, а не косметически: `releases()` теперь симметричен `dispatch()` — оба бросают на `r.status !== 0` и оба ловятся одним и тем же `catch` в `e2eGate`, давая `result: 'error'` с подсказкой по токену. Подтверждено мутацией (откат → тест краснеет), не только чтением. +- `TOKEN_HINT` теперь называет оба репозитория и обе операции — точное описание того, что нужно фолбэк-токену, чтобы не наступить на M1 повторно. +- L4(r3) исправлена текстуально верно: подлежащее (`release.yml`) и дополнение (`houseplan-e2e`/`e2e.yml`) на своих местах. +- Дельта не расширяет скоуп: оба коммита правят ровно то, что назвал вердикт r3 (M1 + L4), новых файлов/модулей не добавлено, кроме публикации самого документа r3. +- Трейлеры обоих коммитов корректны (`Issue: #514`, `User-Visible: no`); `User-Visible: no` по-прежнему верно — правка видна только в тексте ошибки CI-гейта и в `PROCESS.md`, не пользователю продукта. +- База `origin/dev` не сдвигалась между r3 и r4 — условие для сокращённого делта-разбора выполнено, а не предположено. + +## Чего не проверял и почему + +- `tsc --noEmit`, `npm test` (полный), `npm run build` с попиксельной сверкой бандла — зелёный Validate на `0ce5e4ee` уже покрывает (ссылка на прогон дана в условии раунда). +- `golden:verify`, `check-docs.mjs`, `model-invariants.mjs`, `pytest tests_backend`, браузерные смоки — дельта не трогает `src/**`, геометрию/`layout`/толщины или Python; так было во всех четырёх раундах этой задачи. +- Реальный dispatch `release.yml` по событию `release: published` с настоящим `E2E_DISPATCH_TOKEN` — секрет ещё не заведён владельцем (это и был контекст M1 — риск в конфигурации, которая пока не существует), недоступно ревьюеру. Первая живая проверка — следующий stable-релиз, как честно называет ТЗ. +- README `houseplan-e2e` — не перечитывал повторно, вне диапазона `origin/dev...HEAD` во всех раундах, контракт имени job (единственное, от чего зависит код) был сверен напрямую в r3 и код, от которого он зависит, не менялся. + +## Продуктовое рассуждение + +Дельта не меняет продуктовую оценку r3: задача по-прежнему укрепляет цепочку поставки (гейт стабильного релиза дожидается настоящего E2E), в скоуп `docs/SCOPE.md` не входит напрямую (инфраструктура процесса), беты не задевает. M1 был риском в конфигурации, которую владелец ещё не завёл; фикс убирает этот риск заранее, не дожидаясь, пока он реализуется в проде. Ни один AC не ослаблен правкой — только усилен (releases() больше не может тихо соврать). + +## Вопросы владельцу + +Нет открытых продуктовых вопросов. + +--- + +## Материал раунда + +- Ветка: `issue/514-e2e-stable-release-gate`, коммит `0ce5e4eed3d1d2e3a52f07c35dbc9a8297dd01a2`. +- Диапазон: `origin/dev..HEAD` (9 коммитов), `origin/dev` = `ad2858a80cc7be36a8577c8b05ea330dca66eba5`. +- ТЗ: `docs/specs/514-e2e-stable-release-gate.md`; ревью ТЗ: `docs/reviews/SPEC-REVIEW-514-r1.md` (зелёное, r1). +- Предыдущий код-ревью: r3, жёлтый, материал `d166bad3eb293d70cdd37cc715c639449742d988` (осиротел после ребейза; content-эквивалент восстановлен как `a8a9b7d99e61af685149c903d34fab84c1008d90`, см. раздел выше). +- Дельта r3→r4: `git diff a8a9b7d9..HEAD` — 2 коммита (`ed6d1d76` docs-only, `0ce5e4ee` M1+L4 фикс), 3 файла кода/процесса, +24/−5. +- Validate на материале раунда: https://github.com/Matysh/houseplan-card/actions/runs/34406969841 (success, дано условием раунда). +- Вердикт этого раунда: **зелёный** · High 0 · Medium 0 · Low 4 (L1-L3 унаследованы; L4 закрыта фиксом этого раунда; L5 новая, принята без правки). + +--- + + + +## Материал раунда + +- Ветка: `issue/514-e2e-stable-release-gate`, коммит `0ce5e4eed3d1` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `55fd573338326d3dbde36a2e4444439cce493833` + ``` + git log --all --format='%H %T' | grep 55fd57333832 + ``` +- ТЗ `docs/specs/514-e2e-stable-release-gate.md`, блоб `30795778fee11c8df610440dd86b745112e120c4` + ``` + git log --all --find-object=30795778fee11c8df610440dd86b745112e120c4 -- docs/specs/514-e2e-stable-release-gate.md + ``` +- Вердикт конвейера: `green` · High 0