houseplan-card/docs/reviews/CODE-REVIEW-657-r1.md

CODE-REVIEW-657-r1

• Issue: https://github.com/Matysh/houseplan-card/issues/657
• Материал: git diff origin/dev...HEAD на 339379f504f7ab5c9142bbca887c363fc0645f1e (ветка issue/657-generated-at-merge, HEAD detached), 35 файлов, +1021/−136. Якоря совпадают: head=339379f5, tree=b63cbf25…, base=6d3738ad (origin/dev).
• Класс: B (scripts/**, .github/workflows/**, test/**) + C (PROCESS.md, AGENTS.md, docs/**) — класса A нет.
• Этап: код-ревью (PROCESS.md §2.7), заход r1, блокирующих циклов израсходовано 0/4.
• Вердикт: красный.

Скоуп

Задача закрывает инфраструктурную часть SCOPE (конвейер ревью — не пользовательский джоб, а обслуживание J1–J7 опосредованно, через скорость доставки; сама задача это и объявляет как «инфраструктурная, вход сразу на S7»). Решения владельца: 1б — docs/reviews/INDEX.md пересобирается только коммитами, идущими прямо в dev (слияние кандидата, публикация SPEC-REVIEW), а не в ветке задачи; 2б — бандл (dist/**, custom_components/houseplan/frontend/**) в дереве dev меняет только релизный кандидат/бета (трейлер Release:), стенд dev.houseplan.tech берёт бандл головы dev из артефакта Validate через служебную ветку dev-build.

AC из хендоффа — семь строк «AC · чем доказан · чем краснеет» (см. комментарий автора от 2026-09-26T05:59:00Z). Разбор велся по каждой строке отдельно чтением кода и, где это было практично, реальным исполнением (не моками) сценариев, которые юнит-тесты задачи проверяют только частично.

Как проверялось

Дешёвые гейты подтверждены зелёным Validate на этом SHA (https://github.com/Matysh/houseplan-card/actions/runs/36222411365, push) — typecheck, npm test, npm run build + bundle-policy --verify, мутанты (сharded) все прошли. Автор дополнительно прогнал полный node --test test/*.test.mjs локально (шесть частей) и вручную все 11 новых мутантов (assertion-killed). Перегонять эти гейты не стал — правило #343/условие честности сужения.

Дополнительно прогнал сам (не входит в стандартный набор для B/C-диффа, но необходимо для проверки защитного AC «индекс свеж после integrate»):

Гейт	Команда	Результат
smoke-select	node scripts/smoke-select.mjs --base origin/dev --head HEAD	«Исполняемого frontend-диффа нет» — смоки не выбираются, диф не трогает src/**
воспроизведение сценария fast-forward (см. находку H1)	реальный git-репозиторий, оригинальный scripts/reviews-index.mjs без правок, без моков	индекс красится — см. ниже

Не прогонял: golden (npm run golden:verify), pytest tests_backend, performance — диф не трогает demo/** (визуал), custom_components/**/*.py или профили; AC их не называет. check-docs.mjs не обязателен: диф не трогает src/** (§8, REVIEWER.md «Объём гейтов»).

Находки

H1 — после fast-forward слияния (dev не двигался за время ревью) docs/reviews/INDEX.md остаётся неполным, и Validate на dev красится на первом же таком merge

Заявленный AC (таблица хендоффа): «Индекс не пересобирается в ветке задачи; после integrate свеж» — доказательство: test/reviews-index.test.mjs «#635/#657 (1б)», rebase-generated (порядок шагов); краснеет — мутант process-index-on-task-branch.

Что реально происходит. До этой задачи шаг «Опубликовать документ ревью» (_process.yml) пересобирал docs/reviews/INDEX.md тем же коммитом, что и сам документ, независимо от цели (if [ -f "$doc" ]; then … fi) — то есть каждый код-ревью-коммит в ветке задачи уже нёс свежий индекс. Этот диф сузил условие до [ "$target" = "dev" ] (истинно только для SPEC-REVIEW, публикуемого прямо в dev; для CODE-REVIEW цель — ветка задачи, и индекс там больше не трогается — это и есть решение 1б). Расчёт был на то, что merge-candidate.mjs дособерёт индекс при слиянии («слиянием кандидата (merge-candidate.mjs, уже умеет)» — из решения владельца). Но merge-candidate.mjs в этой задаче не менялся (нет в диффе), и пересборка индекса (--commit-if-stale) в нём вызывается только внутри rebaseOnto() — то есть только когда devMoved === true и требуется ребейз. Когда dev не сдвинулся за время ревью (decideMerge({devMoved: false}) → fast-forward, ветка merge-candidate.mjs:250-253), происходит прямой pushWithLease(tip, 'dev', devNow) — индекс не пересобирается вообще. Это не экзотика: это отдельная, специально тестируемая ветка решения (test/merge-candidate.test.mjs: «dev не двигался: push кандидата как есть...»), которая наступает всегда, когда во время ревью текущей задачи ни одна другая задача не слилась в dev (тихие периоды, ночь, единственная задача в очереди — именно то, чего #657 и добивается, снижая частоту параллельных слияний).

Воспроизведение (настоящий git, неизменённый scripts/reviews-index.mjs из этой ветки — без моков, без правок):

git init -q -b dev
# … docs/reviews/CODE-REVIEW-1-r1.md + `reviews-index.mjs` → коммит, индекс свеж
node scripts/reviews-index.mjs --dir=docs/reviews --check   # OK, «индекс свеж»

git checkout -q -b issue/700-x
# добавлен docs/reviews/CODE-REVIEW-700-r1.md, INDEX.md НЕ тронут (= поведение
# публикации документа в ветку задачи после этого диффа)
git commit -q -m "docs: review document for #700"

git checkout -q dev
git merge --ff-only -q issue/700-x       # = decideMerge({devMoved:false}) → fast-forward

node scripts/reviews-index.mjs --dir=docs/reviews --check
# → "docs/reviews/INDEX.md устарел — пересобрать: node scripts/reviews-index.mjs"
# exit code: 1

Это ровно тот же гейт, что стоит в validate.yml на push в dev (id: reviews_index, шаг «Индекс ревью совпадает с каталогом docs/reviews», validate.yml:104-107), и его провал не «continue-on-error» на уровне итогового вердикта: шаг «Вердикт предполётных проверок» (validate.yml:211-234) явно делает check "индекс ревью совпадает с каталогом" "$REVIEWS_INDEX" и exit $fail — красный шаг красит весь preflight job, а значит и весь прогон Validate на голове dev.

Почему это находка именно этой задачи, а не старый долг. До #657 индекс уже приезжал в dev свежим при fast-forward, потому что ветка сама несла самодостаточный INDEX-коммит (см. выше). #657 убрал этот путь для CODE-REVIEW (единственный тип документа, идущий через merge-candidate.mjs), не добавив взамен пересборку в фаст-форвардной ветке слияния. AC таблицы хендоффа проверяет ровно две вещи — что публикация в ветку задачи индекс НЕ трогает (верно) и что merge-candidate.mjs «всё ещё» вызывает --commit-if-stale» (тоже формально верно — вызывает, но не на этом пути) — и не проверяет само условие «после integrateсвеж» для фаст-форвардного слияния, потому чтоtest/merge-candidate.test.mjsработает на fakeOps и не в состоянии увидеть файловые последствия дляdocs/reviews/`.

Чем красится: сценарий выше — реальным git merge --ff-only + reviews-index.mjs --check на неизменённом коде репозитория; это не гипотетическая, а прямо воспроизводимая последовательность действий конвейера при следующем же тихом слиянии.

Что нужно поправить (не мой выбор, но для ориентира владельцу): в decideMerge/mergeCandidate фаст-forward ветка обязана либо тоже дергать reviews-index --commit-if-stale --issue=$issue на tip перед pushWithLease, либо публикация документа в ветке задачи обязана сама включать свежий INDEX-коммит для случая, когда слияние окажется fast-forward (тогда придётся вернуть часть анализа конфликтов, которого 1б избегает). Выбор архитектуры — авторский; здесь фиксируется только то, что текущий код инвариант не держит.

Серьёзность: High (блокирует). AC таблицы хендоффа для этой строки не выполнен на реальном, регулярно достижимом пути; последствие — не тестовый провал в ветке задачи, а красный Validate на dev после штатного слияния, без явной причины в диффе того же дня (следующий разработчик будет разбираться, что сломало dev, не находя в своём коммите ничего relevant).

Что проверено и корректно

• Бандл-политика (2б), правило коммита. scripts/bundle-policy.mjs (bundleCommitErrors, isBundlePath, releaseTrailers) и его подключение в validate-commit-provenance.mjs (и хук commit-msg, и история в CI) — прочитано полностью, логика согласована с test/bundle-policy.test.mjs. Хук судит без даты автора (authorDate не передаётся) — значит, судится всегда, а разграничение по BUNDLE_RELEASE_ONLY_SINCE работает только для ретроспективной проверки истории в CI, что и заявлено («в хуке даты нет — судится всегда»). Проверено чтением и сверкой с тестом, тест умеет падать (мутанты bundle-policy-rule-off, bundle-policy-cutoff-inverted).
• --verify/--must-match (сверка копий только на кандидате). committedBundleMustMatch корректно решает по путям коммита ИЛИ по isCandidateSubject (кандидат, забывший пересобрать бандл, всё равно сверяется) — прочитано, обосновано тестом (bundle-policy-always-compares, bundle-policy-candidate-not-compared). Отдельно проверил вручную: git diff-tree -r --no-commit-id --name-only HEAD для настоящего двухродительского merge-коммита возвращает пустой список (эксперимент в /tmp/mergetest) — то есть commitShape() в этом (маловероятном при линейной модели dev) случае решит «копии не сверяются» даже если бандл менялся. Это не считаю находкой уровня Medium: merge-candidate.mjs не создаёт двухродительских коммитов в dev (только fast-forward/rebase-push), а validateCommitMessage's собственный range-check уже сознательно пропускает parentCount > 1 тем же способом (continue) — новый код просто следует уже принятому в кодовой базе допущению. Фиксирую как наблюдение, не находку.
• assertCommittedBundleFresh / публикация беты. Читает манифест из закоммиченного дерева публикуемого SHA (не с диска), сверяет с sourceFingerprint(root) при чистом дереве на этом SHA — корректно; мутант release-prerelease-skips-fresh-bundle доказывает, что вызов не выпадает молча.
• bundle-sync --release / bundle:clean. TARGETS формируется по наличию --release; без флага HACS-копия не трогается — проверено чтением и тестом test/bundle-sync.test.mjs (реальный spawnSync, не мок). bundle-policy.mjs --clean (git checkout --, git clean -fdq) возвращает ровно корни бандла, не трогая остальное дерево — тест это подтверждает (notes.txt остаётся).
• dev-build.mjs / стенд. publishDevBuild строит коммит без родителя во временном индексе (не трогая рабочее дерево источника), публикует только если expectRef не ушёл вперёд, иначе тихо уступает следующему прогону — прочитано и подтверждено test/dev-build.test.mjs на настоящих git-репозиториях (bare + клон), включая сценарий стенда (update-dev-bundle.sh --reset / без флага). validate.yml: dev_build job условие push && ref == refs/heads/dev, continue-on-error: true, не входит в needs job proof — подтверждено и тестом test/validate-workflow.test.mjs, и чтением самого workflow.
• rebase-on-dev.mjs. Конфликт по бандлу теперь берёт версию dev без пересборки/amend — согласовано с тем, что ветка бандл не несёт; GENERATED_ROOTS теперь читается из bundle-policy.BUNDLE_ROOTS, единая точка. Тесты (test/rebase-on-dev.test.mjs) переписаны без фальшивой пересборки и это корректно отражает новое поведение.
• docs/reviews — переключение публикации на target=dev. Само условие в _process.yml (строка if [ -f "$doc" ] && [ "$target" = "dev" ]; then) и соответствующий мутант (process-index-on-task-branch) корректны как локальное решение; ограничение находки H1 — не в этом условии, а в отсутствующей компенсации на стороне merge-candidate.mjs.
• Второй коммит (339379f5) — понижение базы bundleBytes. Причинно обосновано в теле коммита (Validate на первом коммите падал на храповике связности монолита из-за постороннего дрейфа размера dist после хотфиксов #649, не из-за этой задачи); не расширяет скоуп.
• Трейлеры. Оба коммита несут Issue: #657 и User-Visible: no — корректно: класса A нет, видимого пользователю поведения нет, changelog не требуется.
• Одно число — один источник. Число job-загрузок артефакта card-bundle (было 4, стало 5 — новая dev_build) сверено вручную по самому workflow (grep -n "name: card-bundle" — ровно 5 вхождений: 1 upload + 4 download: dev_build, smoke, и два прочих браузерных job) и совпадает с тем, что проверяет test/validate-workflow.test.mjs.

Чего не проверял

• Golden, pytest, performance-профили — диф не трогает визуал/Python/ профили, AC их не называет.
• check-docs.mjs — не обязателен (диф не по src/**); заявление автора, что этот гейт независимо красный на скриншотах и на базе dev, не перепроверял (не относится к предмету ревью).
• Живой прогон merge-candidate.mjs против настоящего GitHub API — не запускал (нужны токен и реальный репозиторий); H1 доказан на уровне логики и на уровне docs/reviews/INDEX.md-инварианта напрямую, что для дефекта этого типа достаточно и без интеграции с gh.
• Двухродительские merge-коммиты в dev — отмечено выше как воспроизводимое, но не относящееся к используемому в проекте потоку (см. «Что проверено и корректно»); не поднимаю до находки.

Итог

AC "индекс ревью не пересобирается в ветке задачи; после integrate свеж" не выполняется для fast-forward пути слияния — реального, тестируемого и, по всей вероятности, частого исхода decideMerge(). Это ломает гейт "индекс ревью совпадает с каталогом" на dev без видимой причины в последующих диффах. Остальная часть задачи (бандл-политика 2б, стенд dev, переход публикации документа на target=dev) прочитана и перепроверена (частично исполнением) без находок.

Вердикт: красный · заход r1 · блокирующих циклов 0/4 · High: 1 · Medium: 0 → в задаче · Документ: docs/reviews/CODE-REVIEW-657-r1.md

---

Материал раунда

• Ветка: issue/657-generated-at-merge, коммит 339379f504f7 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: b63cbf255cc76f784688d9ad7553444f9a89e337

  git log --all --format='%H %T' | grep b63cbf255cc7
  

• Тело issue: e15e73b22b32d0524d13035dc170c366dd9aa8a038a6ebb5dab193b2868f5043
• Вердикт конвейера: red · High 1