From 1d151b55656e8f06c9db30f31be06f8d1d6b9689 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 24 Sep 2026 02:23:43 +0000 Subject: [PATCH] docs: review document for #633 Issue: #633 User-Visible: no --- docs/reviews/CODE-REVIEW-633-r1.md | 206 +++++++++++++++++++++++++++++ docs/reviews/INDEX.md | 3 +- 2 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 docs/reviews/CODE-REVIEW-633-r1.md diff --git a/docs/reviews/CODE-REVIEW-633-r1.md b/docs/reviews/CODE-REVIEW-633-r1.md new file mode 100644 index 00000000..41f67727 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-633-r1.md @@ -0,0 +1,206 @@ +# CODE-REVIEW-633-r1 + +Issue: #633 · «Локальный контур до push: pre-push-gate включить по умолчанию и +покрыть тестом, sandbox-bootstrap одной командой, test:chunk для лимита 3 мин» +Трек: инфраструктурный (§1 AGENTS.md/PROCESS.md) — ни одного файла класса A. +Заход: r1 · блокирующих циклов израсходовано 0 из 4 (первый заход, бюджет не +тратился). + +## Скоуп + +Диапазон `origin/dev..HEAD`, один коммит `af62bd4c`: + +``` +.githooks/pre-push | 42 ++-- +docs/DEVELOPMENT.md | 51 +++++ +docs/TESTING.md | 31 +++- +package.json | 1 + +scripts/check-inputs.mjs | 1 - +scripts/mutation-registry.mjs | 79 ++++++ +scripts/pre-push-gate.mjs | 423 +++++++++++++++++++++++++++------------- +scripts/sandbox-bootstrap.sh | 191 +++++++++++++++++++ +scripts/test-chunk.mjs | 88 +++++++++ +test/local-contour.test.mjs | 55 ++++++ +test/pre-push-gate.test.mjs | 228 +++++++++++++++++++++++ +test/test-chunk.test.mjs | 51 +++++ +``` + +плюс полная пересборка бандла (`dist/**`, `custom_components/houseplan/frontend/**`, +класс D) — обоснованно: `package.json` — вход общего `sourceFingerprint` +(`scripts/source-fingerprint.mjs:16`), новый npm-скрипт меняет отпечаток даже +без единой строки в `src/**`. Проверено чтением исходника fingerprint-скрипта: +`visualFingerprint` (используемый для скриншотов документации) проецирует +`package.json` только на `dependencies/devDependencies/overrides/browserslist` +и не видит добавление npm-скрипта — поэтому `check-docs` не требуется, что +согласуется с тем, что diff не трогает `src/**`. + +Класс файлов — B/C/D, ни одного A. Инфраструктурный трек применён правильно: +issue вошло сразу в `S7-code-review`, без ТЗ/спек-ревью, как предписывает §1. + +Трейлеры коммита: `Issue: #633`, `User-Visible: no` — на месте и обоснованы +(смены пользовательского поведения нет; продукт не тронут). + +## Как проверялось + +Материал ревью подтверждён побайтово: дерево коммита и блобы из блока +«Материал раунда» в хендоффе автора совпадают с текущим `HEAD` +(`ba618a11a3e742921db077bdd6f888433b746280` и все 11 перечисленных блобов — +проверено `git rev-parse HEAD:` по каждому файлу). + +Дешёвые гейты (`tsc`, `npm test`, `npm run build`) на этом SHA уже подтверждены +зелёным Validate (https://github.com/Matysh/houseplan-card/actions/runs/35945741366) — +не перегонялись целиком. Поверх этого сам прогнал то, что относится к дельте: + +| Гейт | Команда | Результат | +|---|---|---| +| Целевые юниты дельты | `node --test test/pre-push-gate.test.mjs test/test-chunk.test.mjs test/local-contour.test.mjs` | 20/20 pass (12+4+4), включая e2e-тест настоящего `.githooks/pre-push` на временном bare-репозитории | +| Сверка якорей мутантов | `node scripts/mutation-gate.mjs --check` | все ok, включая 7 новых | +| Каждый из 7 новых мутантов | `node scripts/mutation-gate.mjs --id=` × 7 | все «поймано 1 из 1» лично, воспроизведено (см. таблицу AC ниже); `git status --short` после — пусто | +| Новый `any` | `node scripts/no-new-any.mjs --base origin/dev --head HEAD` | «новых any нет» (диф не трогает `src/**`) | +| Покрытие входов | `node scripts/check-inputs.mjs --coverage` | exit 0 | +| Процессный гейт | `node scripts/process-gate.mjs --range origin/dev..HEAD --target-ref refs/heads/issue/633-local-contour` | «пройден, предупреждений 0» (п.8 не выполнялся — как и у автора) | +| Копии бандла | `cmp dist/houseplan-card.js custom_components/.../houseplan-card.js` (+ panel.js, assets.json) | побайтово совпадают | +| Классификация файлов | чтение `CLASS_A/B/C/D` в `scripts/process-gate.mjs:45-68` | совпадает с таблицей AGENTS.md/PROCESS.md §1 дословно — `pre-push-gate.mjs`/`.githooks/**` классифицируются как B, как заявлено | + +### Чего не проверял + +- `npx tsc --noEmit`, полный `npm test` (2892+ тестов), полный `npm run build` — + не перегонял целиком: зелёный Validate на этом точном SHA уже покрывает их + (ссылка выше), diff не трогает `src/**`, `no-new-any` подтвердил ноль новых + `any`. Прогнал только три файла дельты (см. таблицу) — этого достаточно для + дельты. +- `scripts/sandbox-bootstrap.sh` (AC2) — не гонял сам: реальный прогон требует + сеть (`npm pack @sparticuz/chromium`, `npm ci`) и создание отдельного + worktree с Chromium (~150+ МБ), что не укладывается в бюджет ревью и не + является дешёвым гейтом. Автор привёл конкретные тайминги и вывод + (`node demo/smoke_edge_cases.mjs` → `OK`, `Chromium 152.0.7977.0`) — + **принято как отчёт о ручном исполнении, не перепроверено мной исполнением**. + Механическая часть (нет путей владельца/секретов, синтаксис `bash -n`, + биты 100755, имена шагов) покрыта постоянным тестом + `test/local-contour.test.mjs` — это я прогнал (входит в тройку выше). +- `golden:verify`, браузерные смоки полным набором, `pytest tests_backend`, + performance-профили — не требуются: diff не трогает `src/**` и + `custom_components/**/*.py`, видимого рендера нет, `User-Visible: no`. +- `node scripts/check-docs.mjs` — не требуется: отпечаток документации + (`visualFingerprint`) не видит правку `package.json` (см. скоуп выше), а + `src/**` не тронут вовсе. +- `model-invariants` — не требуется, геометрия и `layout`/`marker.space` не + затронуты. +- Реальное состояние `hp-dev` на машине владельца после инцидента (см. ниже) — + вне моей досягаемости, это заявление автора о его локальной машине, не о + дереве в этом ревью. + +## Находка первого живого прогона автора — прочитано, не находка ревью + +Автор сообщает, что при первом реальном push хук унаследовал `GIT_DIR`/ +`GIT_CONFIG_PARAMETERS` от git и юнит-тесты с временными репозиториями +(`git init tmp`) писали в настоящий `.git` клона (`core.bare=true`, коммиты +фикстур, перезапись `refs/heads/dev`). Инцидент локальный, `origin/dev` не +пострадал (проверено автором через `for-each-ref`, подтверждено привязкой +`hp-dev` к прежнему дереву). Фикс — `gateEnv()` снимает все `GIT_*` перед +запуском набора — я лично воспроизвёл и подтвердил мутантом +`pre-push-gate-inherits-git-dir` (откат `env: gateEnv(env)` → `env` красит +тест). Это ровно тот случай, ради которого затевалась задача (защита от +регресса, а не сама регрессия в этом дереве). Не завожу отдельную находку — +дефект уже исправлен в этом же коммите и накрыт мутантом; отдельно отмечаю, +что просьба автора владельцу проверить `hp-dev` адресована человеку, а не +ревью. + +## AC → доказательство → чем краснеет + +| AC | Доказано | Чем краснеет (лично воспроизведено) | +|---|---|---| +| AC1 — push `issue/*` с красным `gate:small` отклоняется хуком; тест на скрипт | `test/pre-push-gate.test.mjs`: чистые `planHook`/`gateMode`/`candidateRefs`/`isDocsOrGeneratedOnly` + e2e на временном bare-репозитории с настоящим `.githooks/pre-push` (красный → отклонён и ветка не доехала; зелёный → доехал; `HP_PREPUSH_GATE=0` → без прогона; дифф только C/D → без прогона) | 6 мутантов — `pre-push-hook-ignores-red-gate-small`, `pre-push-gate-inherits-git-dir`, `pre-push-shell-drops-gate-status`, `pre-push-hook-misses-issue-branches`, `pre-push-docs-filter-swallows-executable-diff`, `pre-push-hook-judges-foreign-tree` — все лично прогнаны, все «поймано 1 из 1» | +| AC2 — `bash scripts/sandbox-bootstrap.sh` на чистой песочнице даёт зелёный `node demo/smoke_edge_cases.mjs` | Отчёт автора о реальном прогоне (тайминги, версия Chromium, `OK`) — **принято, не переисполнено** (см. «Чего не проверял»); статическая часть — `test/local-contour.test.mjs` (лично прогнан) | скрипт не покрыт мутантом — обоснованно (e2e недёшев для CI/ревью), автор явно это записал | +| AC3 — `test:chunk -- N/M` покрывает все файлы ровно по одному разу | `test/test-chunk.test.mjs`: объединение частей = полный список для M ∈ {1,2,3,4,6,7,n,n+3}, детерминизм, ±1 по размеру, неверный `N/M` — исключение | мутант `test-chunk-drops-last-file` — лично прогнан, «поймано 1 из 1» | + +Все три AC доказаны исполняемым кодом; для AC2 доля доказательства — +исполнение автором + статический тест реви­зии, что честно названо в таблице +и в разделе «чего не проверял», а не выдано за полноценный автотест. + +## Что проверено и корректно + +- **Классификация файлов** (`classify()` в `scripts/process-gate.mjs`, + переиспользована в `pre-push-gate.mjs` через импорт, не задублирована) — + совпадает с таблицей классов AGENTS.md §1 дословно. +- **Дизайн отказа для не-HEAD веток** (`foreign` в `planHook`) — при + одновременном пуше нескольких ссылок, где хотя бы одна с исполняемым диффом + не совпадает с `HEAD`, весь пуш отклоняется, а не тихо пропускает проверку + чужого дерева. Разобрано по коду и тестам (`plan({ headSha: B })`) — это + осознанный компромисс в пользу безопасности (fail closed), а не дыра. +- **`gateEnv`** снимает только `GIT_*`, остальное окружение (включая `PATH`, + пользовательские переменные) остаётся нетронутым — проверено юнитом и + чтением. +- **Ручной режим `#343`** (`node scripts/pre-push-gate.mjs` без `--hook`) + функционально не изменён, лишь обёрнут в `manualGate()` + `isMainModule` + + `portableCommand` — читается построчно, дельта чисто механическая + (выделение функции), логика шагов (rebase-check, диффы, смоки, мутанты) + побайтово перенесена. +- **`scripts/check-inputs.mjs`**: снятие записи `NOT_AN_INPUT` для + `pre-push-gate.mjs` корректно, т.к. файл теперь импортируется тестом и + становится вычислимым входом гейта `test`; `--coverage` зелёный. +- **Копии бандла** синхронизированы побайтово (`cmp`), рассинхрона между + `dist/**` и `custom_components/houseplan/frontend/**` нет. +- **`sandbox-bootstrap.sh`** не содержит путей владельца, токенов и + учётных данных (проверено чтением и тестом), помечает шимы маркером и не + трогает настоящий браузер — корректно по описанию. +- **Мутационный реестр**: все 7 новых id уникальны (проверено `grep -c`), + каждый привязан к верному guard-тесту, `--check` проходит по всему реестру. +- **Одно число — один источник**: неприменимо — задача не выводит пользователю + ни одной величины (инфраструктура, `User-Visible: no`). +- **Классы риска §2.6**: async/данные-права/геометрия/визуал/perf — не + применимы (нет продуктового кода, нет видимого поведения); host/input — + применим и именно он был найден и закрыт автором (наследование `GIT_*` от + git-хука) — разобран выше. + +## Находки + +Нет. High: 0, Medium: 0, Low: 0. + +## Материал раунда + +``` +tree ba618a11a3e742921db077bdd6f888433b746280 +commit af62bd4c6490de198d22d9ff7b7f367e55f7584f +blob 52b9623603959bc2ac5f417f5903dd5a2bd8b71d scripts/pre-push-gate.mjs +blob e325f3c9a5557ed92d5f73848181d2a70c12a726 .githooks/pre-push +blob c406d04d763a9d7913f41c6a26c5decee1f240d5 scripts/sandbox-bootstrap.sh +blob c999836aaaf293e28066ebaf440c0b51fc710021 scripts/test-chunk.mjs +blob 3d39099a70f1be802ed31d8c4c08976c00907372 test/pre-push-gate.test.mjs +blob 1f258832f85ec7cdab73ff7b350e2471d7fc15c1 test/test-chunk.test.mjs +blob 5c934d8d7355190d24ca9023bd3ad0b3a19f438f test/local-contour.test.mjs +blob fd95a2dc33c99fcaa16b93baf0f9081c77123131 scripts/mutation-registry.mjs +blob 8c9ff3fb7537cf1171bb3e2b1a9012b6df1a6276 docs/DEVELOPMENT.md +blob 741c42ab7fb1897317186cc57faa8d4e77b22d75 docs/TESTING.md +blob ca38135dbd70ca9a4b50b1fb0cd402207e861404 package.json +``` + +Проверено: `git rev-parse HEAD^{tree}` = `ba618a11a3e742921db077bdd6f888433b746280`; +`git rev-parse HEAD:` для всех 11 файлов выше — совпадают с указанными +блобами. + +## Вердикт + +Зелёный. AC1–AC3 доказаны исполняемым кодом (юниты, e2e на настоящем хуке, +7 лично воспроизведённых мутантов), инфраструктурный трек применён верно, диф +не задевает ни одного файла класса A, трейлеры на месте, бандл синхронизирован. +Единственное отступление от полного автотеста — AC2 (реальный прогон +`sandbox-bootstrap.sh` в живой песочнице) принят по отчёту автора, а не +переисполнен ревьюером — честно и явно так названо, обосновано ценой (сеть, +Chromium, отдельный worktree) и подкреплено постоянным статическим тестом на +то, что можно проверить дёшево (утечки секретов/путей, синтаксис, биты +исполнения). + +--- + + + +## Материал раунда + +- Ветка: `issue/633-local-contour`, коммит `af62bd4c6490` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `ba618a11a3e742921db077bdd6f888433b746280` + ``` + git log --all --format='%H %T' | grep ba618a11a3e7 + ``` +- Тело issue: `28edc1fc028d8c70c846549713ee1fb5f1b12b291725ad92444774299c21d341` +- Вердикт конвейера: `green` · High 0 diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 51be69de..ee1457b5 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,6 +1,6 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1016, issue: 358. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1017, issue: 359. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| @@ -17,6 +17,7 @@ | #635 | [CODE-REVIEW-635-r1.md](CODE-REVIEW-635-r1.md) | code · r1 | 🟡 жёлтый | 1 | 0 | индекс молчаливо теряет находки и врёт числами по текущему | `docs/reviews/CODE-REVIEW-639-r1.md` `CODE-REVIEW-637-r1.md` `docs/reviews/CODE-REVIEW-594-r1.md` `docs/LESSONS.md` | | #635 | [CODE-REVIEW-635-r2.md](CODE-REVIEW-635-r2.md) | code · r2 | 🟡 жёлтый | 1 | 1 | docs/reviews/INDEX.md, зафиксированный в материале ревью, устарел на собственном SHA — …; parseFindings/parseFiles: фолбэк «первая строка тела блока» вырезает начало буллета и п… | `docs/reviews/INDEX.md` `SPEC-REVIEW-625-r1.md` `SPEC-REVIEW-625-r2.md` `CODE-REVIEW-625-r1.md` `CODE-REVIEW-625-r2.md` `process.yml` `test/reviews-index.test.mjs` `INDEX.md` | | #635 | [CODE-REVIEW-635-r3.md](CODE-REVIEW-635-r3.md) | code · r3 | 🟢 зелёный | 0 | 0 | firstParagraph: ветка нет\b в фильтре мёртвая из-за ASCII-only \b в JS-регэкспах, расхо… | `scripts/reviews-index.mjs` | +| #633 | [CODE-REVIEW-633-r1.md](CODE-REVIEW-633-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #630 | [CODE-REVIEW-630-r1.md](CODE-REVIEW-630-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #629 | [SPEC-REVIEW-629-r1.md](SPEC-REVIEW-629-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — | | #627 | [SPEC-REVIEW-627-r1.md](SPEC-REVIEW-627-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | избыточное (не противоречивое) условие в AC2; влияние на touch не названо явным пунктом | `docs/TOUCH-SUPPORT.md` |