diff --git a/docs/reviews/CODE-REVIEW-703-r2.md b/docs/reviews/CODE-REVIEW-703-r2.md new file mode 100644 index 00000000..f47ef701 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-703-r2.md @@ -0,0 +1,187 @@ +# CODE-REVIEW-703-r2 + +Issue: #703 · «stable promotion ложно судит уже проверенную бета-линию» +Трек: show (владелец задал критерии в теле issue) +Материал: `241f8d48f500f09c21ee91f55e34ce552ce2b5b4`, ветка +`issue/703-promotion-range-base` поверх `origin/dev` +Заход: r2 · блокирующих циклов израсходовано 1 из 2 + +## Скоуп + +Инфраструктурная задача (не класс A). Три AC в теле issue не изменились с r1 +(см. CODE-REVIEW-703-r1): AC1 (граница по зелёному прогону на `dev`/тегу), +AC2 (новое нарушение после границы по-прежнему красное), AC3 (контракт +`validate.yml` + PROCESS.md §10.2). Этот раунд — точечный фикс по единственной +находке High из r1, плюс правка Low. User-Visible: no в трейлере — верно, +поведения продукта дифф не меняет. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| **High** — `releaseTaggedShas()` в реальном Validate всегда пуст: ни один checkout не ставит `fetch-tags: true`, а `actions/checkout@…` (v7) при дефолте передаёт `git fetch --no-tags` безусловно | Добавлен `fetch-tags: true` к обоим checkout, читающим диапазон | `.github/workflows/validate.yml:74` (job `preflight`) и `:347` (job `changes`); `test/validate-workflow.test.mjs:372-379` теперь требует точную строку `with: { fetch-depth: 0, filter: 'blob:none', fetch-tags: true }` для обеих job | +| **Low** — тест AC2 «пуш в dev» вызывал `rangeBase` с ИДЕНТИЧНЫМИ аргументами, что и «hotfix на main»: второй ассерт был дубликатом первого | `onDev` теперь получает свой вход — только прогоны `dev` (без `main`) и более старый `fallback` (`fx.beta3` вместо `fx.candidate`), плюс явный `assert.equal(onDev.base, fx.candidate)`, которого раньше не было вовсе | `test/promotion-range.test.mjs:131-134` | + +Проверено исполнением, не только чтением диффа — см. «Как проверялось». + +## Унаследовано из r1 + +Без повторной проверки принято (материал не менялся в дельте r1→r2 — см. +`git diff 4496c232a0a5..HEAD --stat`: тронуты только `validate.yml`, два +тестовых файла и добавленный `docs/reviews/CODE-REVIEW-703-r1.md`): + +- **AC1, первая часть** (SHA, зелёный на `dev`, даёт пустой диапазон) — + доказано исполнением и мутацией в r1 (`CODE-REVIEW-703-r1.md`, раздел «Что + проверено и корректно»). Код `pickRangeBase`/`headGreen`-ветка не менялся в + этой дельте. +- **AC2, оба сценария логики** (упавший HEAD не граница; повтор упавшего SHA + судится тем же диапазоном) — доказано в r1 мутацией и прямым вызовом + `pickRangeBase`; код не менялся. +- **AC3, контракт workflow** (job `changes`/preflight читают обе ветки, + preflight не падает на `event.before` для `main`, PROCESS.md §10.2) — + подтверждено чтением в r1; в дельте r1→r2 эти строки не тронуты (см. + `git diff` ниже — правка ограничена добавлением `fetch-tags: true` и двумя + строками комментария рядом). +- `releaseTaggedShas` корректно разбирает annotated и lightweight теги — + логика функции не менялась, проверено в r1 отдельным прогоном + `git for-each-ref` на реальном репозитории. +- PROCESS.md §10.2 соответствует коду — файл не менялся в этой дельте. + +Материал r1: `4496c232a0a53186b5db426b584b99a431f7cfbb`, вердикт документа — +красный, High 1. + +## Как проверялось + +Прочитаны заново: тело issue #703, оба прежних комментария и вердикт r1 +(«Взял», «Сделано», вердикт конвейера), новый комментарий автора «Сделано +(r2, по ревью r1)». PROCESS.md §2.10 (объём по дельте), §8, §10.2 повторно не +перечитывались — не изменились с r1. + +Дельта разобрана целиком: `git diff 4496c232a0a5..HEAD --stat` — 4 файла +(без учёта опубликованного `docs/reviews/CODE-REVIEW-703-r1.md`, который сам +и есть материал предыдущего раунда): `.github/workflows/validate.yml` (+10/-6 +строк — комментарии и `fetch-tags: true` в двух `with:`), два тестовых файла. +`scripts/classify-base.mjs` и `PROCESS.md` в дельте НЕ участвуют — логика +выбора базы не менялась, только источник данных (checkout). + +Прогнано: +- `node --test test/promotion-range.test.mjs` → `pass 6, fail 0`. +- `node --test test/validate-workflow.test.mjs` → `pass 28, fail 0`. +- `npm run gate:small` → зелёный (99 с; сборка+typecheck, no-new-any, + no-new-private-writes, smoke-select, юниты, bundle-policy, lint:unused). + Дублирует то, что Validate на этом SHA уже подтвердил зелёным + (https://github.com/Matysh/houseplan-card/actions/runs/36756938908), прогнал + сам как дополнительную проверку дельты, не потому что ссылка недостаточна. +- `node scripts/smoke-select.mjs` — не перезапускал отдельно, `gate:small` + включает его в себя: «исполняемого frontend-диффа нет». + +**Мутация на закрытие High** (главная проверка раунда): временно убрал +`fetch-tags: true` из обоих `with: { … }` в `validate.yml` +(`sed` на точную строку), перезапустил `test/validate-workflow.test.mjs` → +`pass 27, fail 1` — новый тест AC3 красится ровно на этой мутации. Откатил +файл из бэкапа, `git status --short` после отката пуст — эксперимент следов +не оставил, тест умеет падать именно там, где закрывает находку. + +Проверил также область поражения находки: `grep` по `validate.yml` показал, +что `classify-base.mjs --mode=range` (путь, которому нужны теги) вызывается +только в двух местах — `preflight` (:214) и `changes` (:392, :398) — и оба их +checkout теперь получили `fetch-tags: true`; остальные ~12 checkout в файле +(job `reuse`, golden, smoke и т.д.) этот путь не используют, править их не +требовалось. + +## Находки + +Нет. High из r1 закрыт и проверен мутацией на этом же материале; Low из r1 +закрыт с более сильным ассертом, чем требовалось для «не блокирует». + +Замечание не на уровне находки: новый вход `onDev` (`{ dev }` без `main`, +`fallback=fx.beta3`) отличается от `onMain` количеством переданных файлов +прогонов, но реальный `validate.yml` всегда запрашивает ОБЕ ветки на обоих +путях (`preflight`: `for branch in dev main`; `changes`: свой + `other`) — +то есть тест «пуш в dev» и «пуш в main» в проде получают одинаково полные +данные, разница только в том, чей SHA судится. Тест это не воспроизводит +буквально, но он и не должен: сам сценарий «резолюция base работает и когда +данные по другой ветке недоступны» (реалистичный случай — `gh api` для +`other` вернул `{}` при сетевой ошибке, что и обрабатывает `|| echo '{}'` в +workflow) — законная, пусть и более узкая, проверка. Это была находка Low +уровня «бухгалтерия» ещё в r1 и решение снять её оставлено на усмотрение +автора; новый вариант строже прежнего (явный ассерт на `base`, не только на +`privateWrites`), поэтому закрытие принимается. + +## Что проверено и корректно + +- Обе строки `with:` в `validate.yml` (:74, :347) идентичны и содержат + `fetch-tags: true` рядом с уже существующим `fetch-depth: 0` — теги и полная + история качаются одним и тем же шагом, лишнего сетевого трафика (blob) не + добавляется, что и заявлено в комментарии кода. +- `test/validate-workflow.test.mjs:372-379` проверяет точную строку `with:` + для ОБЕИХ job (`preflight`, `changes`) — правка одной из двух молча не + пройдёт мимо теста. +- Мутация (см. «Как проверялось») подтверждает: тест краснеет именно на + отсутствии `fetch-tags: true`, а не на чём-то соседнем. +- Скоуп дельты узкий и не расширяет зону риска: `scripts/classify-base.mjs` + не тронут — сама логика выбора базы (то, что уже проверено мутацией и + прогоном в r1) не менялась, изменился только вход (реальный источник + тегов). +- Трейлеры коммита `241f8d48` — `Issue: #703`, `User-Visible: no` — верны, + дифф ограничен CI-инфраструктурой. +- «Одно число — один источник» (§8) — неприменимо, дифф не вводит величин, + видимых пользователю. + +## Чего не проверял + +- Полный `npx tsc --noEmit` / `npm run build` со сверкой трёх копий бандла + отдельно не гонял: `gate:small` включает `npm run build` (typecheck внутри) + и `bundle-policy --verify`, а Validate на этом SHA зелёный + (run 36756938908) — оба основания есть, дублировать полный набор незачем. +- Браузерные смоки — `gate:small` уже прогнал `smoke-select`: «исполняемого + frontend-диффа нет»; Chromium не поднимал, тело issue не называет смок. +- `npm run golden:verify` — метки `ci:golden` нет, `demo/golden/**` не тронут. +- `python -m pytest tests_backend` — `custom_components/**/*.py` не тронут. +- `npm run invariants` — дифф не касается геометрии модели плана. +- Performance-профили — не названы в AC, дифф не трогает рендер. +- Мутанты по диффу — трек `show`, не запрашивались (см. заголовок задачи); + дифф не содержит продуктового кода. +- Живой промоушен `dev → main` на реальном GitHub Actions — по-прежнему не + воспроизведён (это и есть единственный способ доказать находку r1 «в бою», + а не только чтением `dist/index.js` экшена); ближайшая проверка — следующий + настоящий stable-promotion. Не находка: то же ограничение уже зафиксировано + в r1 и автором. + +## Таблица «AC · чем доказан · чем краснеет» (защитные AC) + +| AC | чем доказан | чем краснеет | +|---|---|---| +| AC1 (пустой диапазон на доказанном SHA) | `test/promotion-range.test.mjs`, тест «#703 AC1: промоушен …» | мутация на HEAD-проверку (доказано в r1, код не менялся) | +| AC1 (тег как граница без прогонов dev) | тот же файл, тест «#703 AC1: без прогонов dev …» + для продакшн-пути — `test/validate-workflow.test.mjs` assertion на `fetch-tags: true` | мутация `sed` этого раунда: убрал `fetch-tags: true` → `validate-workflow.test.mjs` падает (pass 27, fail 1); юнит-тест на фикстуре краснеет мутацией из r1 | +| AC2 (новое нарушение красное) | тесты «#703 AC2: …» ×2, второй сценарий теперь с собственным входом | отрицательный случай внутри теста (доказано в r1, код не менялся) | +| AC3 (workflow-контракт) | `test/validate-workflow.test.mjs`, тест «#703 AC3» | строковые ассерты на точный `with:` — точечно, но соразмерно контракту YAML | + +## Вердикт + +Зелёный. High из r1 (release-tag граница неработоспособна в реальном Validate +из-за отсутствия `fetch-tags: true`) закрыт точечно — добавлен ровно в те два +checkout, что используют `classify-base.mjs --mode=range`, — и проверен +мутацией на этом материале: тест краснеет при откате правки. Low из r1 +(дублирующий ассерт в тесте AC2) закрыт с более сильной проверкой, чем +требовалось. Дельта r1→r2 не расширяет риск — логика выбора базы +(`scripts/classify-base.mjs`) не менялась, менялся только вход. Все три AC +задачи выполнены и доказаны: AC1/AC2 логикой и мутациями (унаследовано из r1 +там, где код не менялся, дополнительно подтверждено в r2 там, где менялся), +AC3 контрактом workflow. `gate:small` зелёный, Validate на этом SHA зелёный. + +--- + +--- + + + +## Материал раунда + +- Ветка: `issue/703-promotion-range-base`, коммит `241f8d48f500` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `f0fa1b457febbf21e510dd2b88305134928cb23a` + ``` + git log --all --format='%H %T' | grep f0fa1b457feb + ``` +- Тело issue: `d3b8c4a60a2666a458283fb684284710ad5982fe267e8a362f852f3dedaa5761` +- Вердикт конвейера: `green` · High 0