Files
2026-09-24 02:23:43 +00:00

17 KiB
Raw Permalink Blame History

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:<path> по каждому файлу).

Дешёвые гейты (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=<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:<path> для всех 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