From d20ef60037a269c86f4fa55578968196a2fa34c5 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 9 Sep 2026 18:50:48 +0000 Subject: [PATCH] docs: review document for #514 Issue: #514 User-Visible: no --- docs/reviews/SPEC-REVIEW-514-r1.md | 91 ++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-514-r1.md diff --git a/docs/reviews/SPEC-REVIEW-514-r1.md b/docs/reviews/SPEC-REVIEW-514-r1.md new file mode 100644 index 00000000..f9caceba --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-514-r1.md @@ -0,0 +1,91 @@ +# SPEC-REVIEW-514-r1 + +- **Issue:** https://github.com/Matysh/houseplan-card/issues/514 — «E2E на реальном HA — гейт стабильного релиза» +- **ТЗ:** `docs/specs/514-e2e-stable-release-gate.md` +- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4 (первый заход, бюджет ещё не тратился — #227) +- **Материал:** тело issue #514 + комментарий S2-аналитики (Codex, 2026-09-09, dev `fa01aa00`) + `docs/specs/514-e2e-stable-release-gate.md` на SHA `ea917603132e379013c70f77b802dcf25e06614e` +- **Вердикт: зелёный** + +## Скоуп ревью + +Задача помечена `infra`/`process`, автор явно назвал критерий §5, который трек не проходит («две поверхности — `release.yml`+скрипт в `houseplan-card`, `e2e.yml` в `houseplan-e2e` — плюс секрет владельца»), поэтому полный трек и файл в `docs/specs/` оправданы, а не выбраны по умолчанию без causa. Проверялись: обязательные разделы §7.1, однозначность и доказуемость каждого AC, отсутствие выданных за решения догадок, отсутствие продуктовых вопросов владельцу, которые ревьюер обязан снять сам. + +## Как проверялось + +Проверка велась не на веру к тексту ТЗ, а сверкой с фактическим состоянием обоих репозиториев — большинство утверждений автора о существующем коде оказались проверяемыми напрямую: + +1. `PROCESS.md` §1–14 (жизненный цикл, лимит циклов, лёгкий трек, артефакты §7.1, гейты §8, запрещено §12) и `docs/SCOPE.md` — рамка и процесс. +2. `.github/workflows/release.yml` — текущая структура job `gate`/`build`, ровно та точка, куда ТЗ предлагает вставить новый шаг (после «Require full performance for a stable release», тот же `if: !prerelease`). +3. `scripts/validate-gate.mjs` и `scripts/merge-candidate.mjs` — образец, на который ссылается ТЗ (`VALIDATE_APPEAR_MS = 3 мин`, `VALIDATE_TOTAL_MS = 45 мин` подтверждены построчно), и паттерн чистой функции над инъектируемыми `ops`, который `e2e-gate.mjs` предлагается повторить. +4. `scripts/mutation-gate.mjs` — нашёл уже существующие мутанты по `validate-gate.mjs` (`review-starts-on-red-validate`, `review-returns-task-on-cancelled-dispatch`, `review-trusts-push-run-without-mutants`) в формате `{id, guard, because, patches:[{file, find, replace}]}`. Два мутанта, заявленные в §7 ТЗ (`release-ships-on-red-e2e`, `release-trusts-foreign-e2e-run`), — прямые аналоги уже работающего паттерна на соседнем скрипте, не выдумка. +5. `docs/DEVELOPMENT.md` (строка про Full Performance, ~371–373) и `PROCESS.md` (строка 703, «Гейт стабильного релиза») — точные места правки, названные в §8 ТЗ, существуют и содержат именно тот текст, к которому ТЗ предлагает дописать «+ E2E». +6. `test/performance-workflow.test.mjs`, `test/validate-workflow.test.mjs`, `test/release-contract.test.mjs` — подтверждают, что тестирование содержимого `.github/workflows/*.yml` через `readFileSync` + `assert` уже рабочий паттерн в этом репозитории; план тестов ТЗ (`test/release-workflow.test.mjs`) технически реализуем этим же способом. +7. **`houseplan-e2e/.github/workflows/e2e.yml`** — получен напрямую через `gh api repos/Matysh/houseplan-e2e/contents/...` (репозиторий владельца доступен для чтения). Сверены все технические утверждения §6 ТЗ построчно: + - имя job `"${{ matrix.suite }} · HP ${{ matrix.ref }} · HA ${{ matrix.ha }}"` — подтверждено, совпадает с контрактом опознания в §4/§12 ТЗ; + - `concurrency: { group: e2e-${{ github.ref }}-${{ github.event_name }}, cancel-in-progress: false }` — подтверждено буквально; утверждение «отмена — рукотворная» в §4 п.3 ТЗ не догадка, а прочитанный факт; + - `matrix.include` для `journeys-dev` (`ref: dev`) сейчас **не имеет условия** — значит на сегодняшний день этот сьют реально гоняется и на `workflow_dispatch`, что подтверждает саму проблему, которую чинит §6 ТЗ (сьют против `dev` красит гейт стабильного тега). Предлагаемое условие `if: matrix.suite != 'journeys-dev' || github.event_name == 'schedule'` — валидный паттерн (job-level `if` может читать `matrix.*` для матричных job) и действительно закрывает найденную проблему; + - `suite: upgrade` берёт `ref: ${{ inputs.upgrade_from || 'stable' }}` (не `houseplan_ref`) — то есть job этого сьюта в принципе не всегда содержит `HP ` в имени. Не дефект: опознание в §4 п.2 требует «хотя бы одна» job с `HP ` (её дают `journeys` и `first-run`), а сам сьют `upgrade` по назначению ставит **старую** версию и обновляется до цели внутри теста (`HOUSEPLAN_REF`, job-level env, читает `inputs.houseplan_ref` независимо от `matrix.ref`) — именно это описано в §6 ТЗ как «upgrade — со stable (предыдущий) на тег», без противоречия. +8. `.github/workflows/release-zip.yml`, `.github/workflows/publish-prerelease.yml` — подтверждают заявление §5 ТЗ: `houseplan.zip` собирается и прикладывается к релизу этими workflow независимо от нового гейта, то есть E2E действительно ставит те же байты, что скачает HACS. + +## Находки + +Не найдено ни одной High- или Medium-находки. Технические утверждения ТЗ о текущем состоянии `houseplan-card` и `houseplan-e2e` проверены построчно и подтвердились; ни одна не оказалась выданной за факт догадкой. + +Low, снятые без правки (не искажают AC и не создают риска): + +- **L1.** §4 использует переменную `t0` без явного объявления («кандидаты с `createdAt ≥ t0 − 60 с`») — по контексту это момент вызова `ops.dispatch(tag)`, аналогично `dispatchedAt` в `validate-gate.mjs`. Это техническая деталь реализации, которую §7.1 прямо разрешает решать исполнителю без продуктового вопроса владельцу («всё, чего пользователь не наблюдает, агенты решают сами»); снимается без правки ТЗ. +- **L2.** Тайм-аут `totalMs = 45 мин` переиспользован из `merge-candidate.mjs`, а не выведен из собственного `timeout-minutes: 40` job'ов `e2e.yml`; даёт 5 минут запаса поверх внутреннего лимита job'а, чего достаточно. Переиспользование готовой, уже проверенной константы — сознательный выбор автора (сам ТЗ называет источник), не находка. + +## AC — проверка на выполнимость и доказуемость + +Все шесть AC пронумерованы, формулируют проверяемое условие и называют способ доказательства (тест/мутант/условие в yml/живой прогон в хендоффе): + +| AC | Доказательство названо | Проверяемо | +|---|---|---| +| AC1 | `test/e2e-gate.test.mjs`, `test/release-workflow.test.mjs` | да | +| AC2 | тест + мутант `release-trusts-foreign-e2e-run` | да, паттерн мутанта подтверждён на аналоге (`review-trusts-push-run-without-mutants`) | +| AC3 | условие в yml + тест | да | +| AC4 | коммит в `houseplan-e2e` + живой dispatch на реальном теге в хендоффе | да — единственный практичный способ для факта в чужом репозитории без своего процесса; честно назван как ручная/живая проверка, а не автотест | +| AC5 | штатный `mutation-gate.mjs` раннер | да | +| AC6 | ревью документации + `User-Visible: no` | да | + +Ни один AC не выдаёт предположение за решение: раздел «12. Принятые предположения» отдельно и честно называет два места, где решение принято агентами без владельца (scope токена `HP_PROCESS_TOKEN`, стабильность контракта имени job), и это ровно тот класс вопросов, которые §7.1 разрешает не выносить владельцу. + +## Продуктовые вопросы владельцу + +Открытых продуктовых вопросов нет. Единственный вопрос из тела issue («нужен ли `E2E_DISPATCH_TOKEN`») — технический (scope токена), не продуктовый (не о том, что видит или делает человек), и он снят в самом ТЗ явным решением с фолбэком и понятным сообщением об ошибке, а не эскалирован — верно по правилу §7.1 «владельцу задаются только продуктовые вопросы». + +Раздел 1.2 «Что человек увидит до и после» отвечает на оба обязательных вопроса §7.1 (кто, где, что видит) одной фразой без терминов реализации: пользователь HACS не видит ничего дополнительного (кроме отсутствия сломанного stable), владелец видит один дополнительный шаг гейта и ссылку на прогон. + +## Обязательные разделы ТЗ (§7.1) + +Все присутствуют: сценарий (§1.1) · что увидит человек (§1.2) · проблема (§1) · скоуп/не-скоуп (§2/§3) · контракт поведения (§4–§6) · UX/модель данных/i18n (§10.0, законно свёрнуто в одну строку — изменений нет) · критерии приёмки с доказательством (§10) · план автотестов (§7) · риски (§10.1) · откат (§9) · release-артефакты (§8, §11). + +## Что не проверялось и почему + +- Не запускались автотесты и гейты (`npm test`, `tsc`, `build`) — на этапе ревью ТЗ кода ещё нет, шаг относится к код-ревью (§2.7), не к ревью ТЗ (§2.4). +- Не проверялась фактическая работоспособность `HP_PROCESS_TOKEN`/`E2E_DISPATCH_TOKEN` на реальном dispatch — секреты недоступны ревьюеру и это явно вынесено в хендофф как первая живая проверка (§5 ТЗ, «проверка — первый stable после слияния»); ТЗ этого не скрывает. +- Коммит в `houseplan-e2e` (изменение условия `journeys-dev`) ещё не существует — это ожидаемо для этапа ТЗ (задача полного трека, две поверхности), сам факт зафиксирован в скоупе (§2 п.3) и будет предметом код-ревью через ссылку в хендоффе, а не этого ревью. + +## Материал раунда + +- SHA ветки: `ea917603132e379013c70f77b802dcf25e06614e` +- Файл ТЗ: `docs/specs/514-e2e-stable-release-gate.md` +- Тело issue #514 и комментарий S2-аналитики (id `IC_kwDOTOcLQM8AAAABTjOGtQ`) на момент ревью. + +--- + + + +## Материал раунда + +- Ветка: `issue/514-e2e-stable-release-gate`, коммит `ea917603132e` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `5c67679a7f74498659849400221ca05fc4c5b2e9` + ``` + git log --all --format='%H %T' | grep 5c67679a7f74 + ``` +- ТЗ `docs/specs/514-e2e-stable-release-gate.md`, блоб `6c98b844b36b81cf483f570e4a38faf8d24f2056` + ``` + git log --all --find-object=6c98b844b36b81cf483f570e4a38faf8d24f2056 -- docs/specs/514-e2e-stable-release-gate.md + ``` +- Вердикт конвейера: `green` · High 0