diff --git a/docs/reviews/CODE-REVIEW-638-r1.md b/docs/reviews/CODE-REVIEW-638-r1.md new file mode 100644 index 00000000..38faa018 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-638-r1.md @@ -0,0 +1,179 @@ +# CODE-REVIEW-638-r1 + +Issue: #638 — «Процесс: независимое ревью "с нуля" перед каждым стабильным +релизом (правило PROCESS.md §11)». +Материал: `0ba81a994bbf9c034d15a514c80311f5d8673156` (один коммит поверх +`dev` `d35a52a3`), рабочая копия на нём. Заход r1. + +## Скоуп + +Инфраструктурная задача (класс A не задет — только `PROCESS.md`, +`.github/workflows/**`, `scripts/**`, `test/**`, docs), маршрут §1 #562: +код-ревью без ревью ТЗ. Добавляет шаг «независимое ревью линии перед +стабильным релизом» (PROCESS.md §11.5): отдельный workflow +`release-review.yml`, вход `scripts/release-review.mjs`, job +`independent-review` в `release.yml`, документация (`AGENTS.md`, +`docs/DEVELOPMENT.md`, `docs/process/REVIEWER.md`), тесты и четыре новых +мутанта в `scripts/mutation-registry.mjs`. + +AC по хендоффу автора (редакция после ответа владельца о неблокирующем +ревью): +- AC1 — `PROCESS.md` §11.5 описывает вход (issue линии по трейлерам, + диапазон «прошлый стабильный..кандидат»), выход + (`docs/reviews/RELEASE-REVIEW-vX.Y.Z.md` в `dev`), правило «не блокирует». +- AC2 (редакция) — `release.yml` ставит ревью в очередь параллельно + гейтам, отдельным job без зависимостей от него у остальных job; модель без + права записи; документ публикует детерминированный шаг. +- AC3 — первый прогон на линии v1.78.0. + +## Как проверялось + +Гейты уже подтверждены зелёным Validate на этом SHA +(https://github.com/Matysh/houseplan-card/actions/runs/36121095964) — +`tsc --noEmit`, `npm test`, `npm run build`+bundle-sync не перегонял. +Дополнительно к этому лично прогнал и перепроверил: + +| Гейт/проверка | Результат | Комментарий | +|---|---|---| +| `node --test test/release-review.test.mjs` | 7/7 pass | базовый прогон | +| `node --test test/release-workflow.test.mjs` | pass | базовый прогон, включая новый `#638 AC2` | +| `node --test test/process-digests.test.mjs` | 5/5 pass | REVIEWER.md/PROCESS.md ссылки синхронны | +| 4 мутанта из диффа `mutation-registry.mjs` | все 4 KILLED | патчил вручную и откатывал (см. таблицу ниже) | +| `node scripts/mutation-gate.mjs --check` | ok для всех 4 новых id | якоря валидны, дублей нет | +| `node scripts/action-pins.mjs` | ok | все внешние Actions — полный SHA | +| `python3 -c yaml.safe_load(...)` на обоих workflow | OK | YAML валиден | +| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | «Исполняемого frontend-диффа нет» | `src/**` не тронут — смоки выбирать нечего | +| `git log -1 --format=%H%n%s%n%b` | `Issue: #638`, `User-Visible: no` | трейлеры на месте, changelog не нужен | + +golden/pytest/invariants/performance не прогонял — diff не трогает рендер, +Python или геометрию модели (не применимо по диффу). + +## Проверка защитных AC мутациями (лично воспроизведено) + +| AC | Чем доказан | Чем краснеет — прогнал сам | +|---|---|---| +| AC1: база линии — прошлый **стабильный** тег, не бета и не хвост | `test/release-review.test.mjs` («беты линии не сжимают диапазон») | мутант `release-review-base-accepts-beta`: заменил `filter` на принимающий беты и `<=` — `node --test test/release-review.test.mjs` → 1 fail (KILLED), откатил | +| AC2: ни один job выпуска не ждёт ревью | `test/release-workflow.test.mjs` `#638 AC2` | мутант `release-waits-for-line-review`: вернул `needs: candidate` → `needs: [candidate, independent-review]` в `gate` — 1 fail (KILLED), откатил | +| модель без права GitHub-записи | `test/release-review.test.mjs` (`--allowedTools` без GitHub-инструментов) | мутант `release-review-model-gets-github-tools`: добавил `,mcp__github__add_issue_comment` — 1 fail (KILLED), откатил | +| повтор на тег с готовым документом не тратит модель | `test/release-review.test.mjs` (`if: needs.prepare.outputs.proceed == 'true'`) | мутант `release-review-reruns-existing-doc`: заменил проверку на `if false` — 1 fail (KILLED), откатил | + +Рабочая копия после проверок чистая (`git status --short` пуст) — откаты +применены полностью на все 3 изменённых файла. + +## Что проверено и корректно + +- **`previousStableTag`/`buildLineMembership`** (`scripts/release-review.mjs`): + база линии строго «наибольший стабильный < tag», не текущий тег, не бета; + сравнение версий числовое (`v1.9.0` < `v1.10.0`), не строковое. Issue линии + берутся только по трейлерам `Issue: #NN` через тот же построитель, что + `RELEASE-MEMBERSHIP.json` беты (#547) — упоминание номера в тексте коммита + без трейлера не считается (тест это явно проверяет отдельным коммитом + `#999`). +- **`productFiles`** переиспользует `classify()` из `process-gate.mjs` + (класс A) вместо собственного regex по путям — единый источник истины, + бандл (`dist/**`, `custom_components/**/frontend/**`) и доки корректно + отсекаются. +- **Разделение прав по job** в `release-review.yml`: потолок workflow — + `contents: read`, нигде во всём файле нет `: write` (проверено `grep`); + `model_review` явно передаёт `github_token: secrets.GITHUB_TOKEN` — без + этого `claude-code-action` получает собственный App-токен с правом записи + (#556, тот же приём, что в `_process.yml`); токен job'а всё равно ограничен + job-level `permissions: contents: read`, так что даже гипотетическая + GitHub-MCP-команда не смогла бы писать — `--allowedTools` без + GitHub-инструментов работает как второй, независимый слой защиты, а не + единственный. +- **`independent-review` в `release.yml`**: `needs: candidate`, + `continue-on-error: true`, `permissions: actions: write` — единственное + право, без унаследованного `contents: write` workflow-уровня (job-level + permissions замещают, а не расширяют дефолт). Условие + `needs.candidate.outputs.prerelease != 'true'` корректно пропускает беты + (проверил, что `candidate` job действительно публикует output `prerelease` + — да, `steps.resolve.outputs.prerelease`). Отказ `gh workflow run` + (`exit 1`) гасится `continue-on-error`, не валит релиз — подтверждено + чтением семантики GitHub Actions, не исполнением реального workflow (сеть + недоступна ревьюеру). +- **Каскад skip в `release-review.yml`**: `publish` не имеет явного `if`, но + по умолчанию `needs: [prepare, model_review]` требует `success()` по всей + цепочке; когда `prepare` рано выходит с `proceed=false`, `model_review` + получает `if: needs.prepare.outputs.proceed == 'true'` → `skipped`, и + `publish` по умолчанию тоже пропускается (skip каскадом, стандартное + поведение GitHub Actions, не изобретение этой задачи) — значит «повтор не + тратит модель» не публикует и не переписывает документ повторно. Проверено + чтением, не исполнением (нет доступа к живому прогону workflow). +- **`review-doc-guard.mjs`** используется в `publish` дважды — по staged-диффу + и по итоговому `origin/dev...HEAD` — ограничивая пуш строго `docs/reviews/` + (тот же guard, что уже защищает основной конвейер, #365). +- **Трейлеры коммита**: `Issue: #638`, `User-Visible: no` — оба на месте; + changelog не требуется, видимого пользователю поведения нет. +- **Пины Actions**: все `uses:` в обоих файлах — SHA полной длины с + версией в комментарии; `action-pins.mjs` подтверждает. +- **YAML обоих workflow** разбирается `yaml.safe_load` без ошибок. + +## Находки + +Нет находок High или Medium в скоупе или вне скоупа. + +**Low (снимаю с записью, не блокирует).** `scripts/reviews-index.mjs` +не знает формата `RELEASE-REVIEW-vX.Y.Z.md` (`DOC_NAME` — только +`CODE|SPEC-REVIEW-…`), поэтому опубликованный документ попадёт в футер +«Вне схемы имён (не индексируются)» вместо табличной строки с +вердиктом/находками. Это не дефект AC1–AC3 этой задачи (индекс их не +касается) и явно раскрыто автором в разделе «Чего не проверял» хендоффа — +корректное, ожидаемое поведение существующего скрипта (проверил код: +`indexEntry` → `parseDocName` → `null` → строка попадает в `skipped` → +`renderIndex` печатает её отдельной строкой, не молча теряет). Оставляю без +действия: если поиск по релизным ревью в табличном виде понадобится — +отдельная задача с собственным AC, не расширение скоупа #638. + +## Чего не проверял + +- Живой прогон `release-review.yml` в CI (нет сети/токенов у ревьюера; + автор явно то же самое пометил как непроверенное — первый штатный прогон + предполагается на v1.78.0). +- AC3 буквально («первый прогон — на линии v1.78.0») — недоказуемо до + фактического релиза; это ожидаемое свойство инфраструктуры, выпущенной + заранее, а не пробел в задаче. +- `golden:verify`, `pytest tests_backend`, `npm run invariants`, + performance-профили — не применимы по диффу (нет рендера/Python/геометрии + в изменённых файлах), поэтому не прогонял. +- Полный `npm test`/`tsc`/`build` заново — сошлись на зелёном Validate этого + SHA, не перегонял. + +## Итог + +AC1–AC2 доказаны автотестами; для всех четырёх защитных AC лично +воспроизвёл мутацию и убедился, что тест краснеет, затем откатил. AC3 +недоказуем до релиза линии v1.78.0 — ожидаемо и явно так отмечено автором, +не находка. Дизайн минимальных прав (job-level permissions, отсутствие +`: write` во всём `release-review.yml`, `continue-on-error` без `needs` у +остальных job) корректен и соответствует решению владельца «ревью не +блокирует выпуск». Единственное наблюдение — Low, не в скоупе AC, снято с +запиской. + + +### Материал раунда +``` +tree b96815171eeda68c6a5b896e35d13565419ebe2b +blob 1e5f0b4071d95984242c04006f30c6bce75e730f .github/workflows/release-review.yml +blob f488fa127a86d0003e6a89d6986cbfae1a0d0937 .github/workflows/release.yml +blob f1d2e66c3a6c75d197d83bdaa7c2c77754cf2188 scripts/release-review.mjs +blob c02aab1975fe2440ac9a7d1cab7aff9e3d25872c test/release-review.test.mjs +blob 68d10cc8ab9ed77e79f8fdabd06db4690aeb7dcb test/release-workflow.test.mjs +blob a75cec68bdda83dad37debe44552bcf5c3eeb792 PROCESS.md +blob 851d82262a5f06fd49f6b2da3b3faa299916f5db docs/process/REVIEWER.md +blob 1305ad4bf684d6395e4f2b3f7caf6cf521cb484e scripts/mutation-registry.mjs +``` + +--- + + + +## Материал раунда + +- Ветка: `issue/638-release-review`, коммит `0ba81a994bbf` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `b96815171eeda68c6a5b896e35d13565419ebe2b` + ``` + git log --all --format='%H %T' | grep b96815171eed + ``` +- Тело issue: `50ad75f437343ca0c69e49e1d91cc05bebd7ee0f147801c45ed6b9d2d3636515` +- Вердикт конвейера: `green` · High 0 diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 1aedc592..d824d57a 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,6 +1,6 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1048, issue: 368. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1049, issue: 369. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| @@ -23,6 +23,7 @@ | #640 | [CODE-REVIEW-640-r1.md](CODE-REVIEW-640-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #639 | [CODE-REVIEW-639-r1.md](CODE-REVIEW-639-r1.md) | code · r1 | 🟡 жёлтый | 0 | 1 | Новая запись smoke-links.mjs для smoke_space_settings_form.mjs описывает проверку, кото… | `smoke-links.mjs` `smoke_space_settings_form.mjs` `scripts/smoke-links.mjs` `demo/smoke_space_settings_form.mjs` `src/editors/space-form.ts` `form-kit.ts` | | #639 | [CODE-REVIEW-639-r2.md](CODE-REVIEW-639-r2.md) | code · r2 | 🟢 зелёный | 0 | 0 | — | — | +| #638 | [CODE-REVIEW-638-r1.md](CODE-REVIEW-638-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #637 | [CODE-REVIEW-637-r1.md](CODE-REVIEW-637-r1.md) | code · r1 | 🟡 жёлтый | 0 | 1 | ложный «—» вместо настоящего «0 ч» в медианах | `scripts/process-metrics.mjs` | | #637 | [CODE-REVIEW-637-r2.md](CODE-REVIEW-637-r2.md) | code · r2 | 🟢 зелёный | 0 | 0 | — | — | | #636 | [CODE-REVIEW-636-r1.md](CODE-REVIEW-636-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |