Files
2026-09-30 18:27:54 +00:00

16 KiB
Raw Permalink Blame History

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