mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-01 12:18:51 +00:00
claude[bot]
@@ -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 зелёный.
|
||||
|
||||
---
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/703-promotion-range-base`, коммит `241f8d48f500` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `f0fa1b457febbf21e510dd2b88305134928cb23a`
|
||||
```
|
||||
git log --all --format='%H %T' | grep f0fa1b457feb
|
||||
```
|
||||
- Тело issue: `d3b8c4a60a2666a458283fb684284710ad5982fe267e8a362f852f3dedaa5761`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user