houseplan-card/docs/reviews/SPEC-REVIEW-543-r1.md

SPEC-REVIEW-543-r1

Issue: #543 · Этап: spec (PROCESS.md §2.4) · Заход: r1 · блокирующих циклов 0/4

Материал: тело issue #543, раздел ## ТЗ, на момент ревью (метка S4-spec-review, два предыдущих комментария — S2-аналитика и передача ТЗ на ревью, оба того же автора, без предыдущих раундов ревью). Это первый заход, разбор полный, без раздела «Унаследовано».

Продуктовая постановка (аудиторский блок ## Пользовательская проблема … ## Состояние при заведении) и ## ТЗ — согласованы: ТЗ прямо цитирует те же строки кода и тот же механизм (_reloadConfigOnly / adoptAuthoritativeGated), без расхождений.

Скоуп ревью

Проверялось: полнота обязательных разделов §7.1, однозначность и доказуемость каждого AC, соответствие продуктовой рамке docs/SCOPE.md (J1 «актуальное состояние», J6 «согласованность нескольких клиентов»), фактическая проверка технических утверждений ТЗ по коду на точном SHA 6d2facc2e37724a6e6e1a96c5e026aa24b595a07 (рабочая копия репозитория стоит на этом коммите — сверено git rev-parse HEAD).

Продуктовых вопросов к владельцу нет и не требовалось: сценарий и видимое поведение зафиксированы однозначно («До/После»), а всё техническое оформлено отдельным блоком «Принятые технические предположения — можно менять на ревью». Технического спора с автором, требующего отдельного цикла из-за разногласия, нет — единственная находка ниже касается точности ссылки на тестовый артефакт, а не архитектуры решения.

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

1. Прочитаны docs/SCOPE.md, AGENTS.md, PROCESS.md §2.4/§2.9/§7.1/§7.2/§4.
2. Прочитано тело issue #543 целиком, включая исходный аудиторский блок (источник, воспроизведение, причина по коду, ожидаемый результат) и оба комментария (S2-аналитика с приоритетом P2 и передача ТЗ на ревью).
3. Прочитан текущий код на названном SHA и построчно сверен с ключевыми техническими утверждениями ТЗ:
   • src/houseplan-card.ts:4417,4466-4501 — _reloadConfigOnly: observedRev сравнивается с _cfgRev только на входе (:4473), дальше идёт await this._getAuthoritativeConfig() и await this._adoptAuthoritative(...) без повторной проверки владения после первого await — подтверждает «Подтверждённую причину» ТЗ дословно;
   • src/config-adoption.ts:428-460 — adoptAuthoritativeGated: await host._signer.prepareImage(...) (:439) идёт до adoptStructuralResponses (:451) без промежуточной проверки заявки — подтверждает К2 буквально;
   • все шесть вызывающих _adoptAuthoritative/adoptAuthoritativeGated мест (houseplan-card.ts:4306,4485, summary-panel-runtime-loaded.ts:630, houseplan-onboarding-runtime.ts:477, houseplan-editor-runtime.ts:8693,9577,9737) уже проверяют status !== 'adopted', то есть новый статус superseded естественно поглощается существующей веткой без правки самих call sites — подтверждает заявление ТЗ «менять call sites только если явная обработка нужна»;
   • все _reloadConfigOnly(true) call sites (houseplan-card.ts:5267,7933, summary-panel-runtime-loaded.ts:671, houseplan-onboarding-runtime.ts:287,410,497, space-copy-runtime.ts:224,248, houseplan-editor-runtime.ts:1842,8623,8703,9552) — подтверждает, что «force/restore» путей на практике много, К7 не придумана под единственный случай;
   • docs/ARCHITECTURE.md:1375-1393 (#500) — «Config/layout identity has one owner», единая точка adoptAuthoritativeGated — центр, в котором ТЗ предлагает разместить общую проверку заявки, соответствует уже установленному инварианту, а не вводит второй путь принятия;
   • прецедент generation-guard уже существует в кодовой базе: _liveSyncGeneration/_liveSyncConnection (houseplan-card.ts:948-949, 4376-4405) — подтверждает, что предлагаемый механизм claim/generation не изобретение с нуля, а расширение уже принятого паттерна;
   • «route leave» — установленный в коде термин (houseplan-card.ts:2059,2752, 7447), а не придуманный ТЗ;
   • test/config-adoption-ownership.test.mjs — прочитан целиком: это статический regex-сканер по src/** (IDENTITY_WRITE, BODY_WRITE), ратчет-тест другого инварианта #500 («только ConfigAdoption пишет _cfgRev/_layoutRev/fingerprint»), см. находку ниже;
   • demo/smoke_save_race.mjs — образец существующего паттерна: смок вызывает приватный c._reloadConfigOnly() напрямую через page.evaluate и подменяет hass.callWS; подтверждает техническую реализуемость AC2/AC4 без новых инструментов;
   • package.json — gate:small, invariants, docs:accept, golden:verify существуют как названо в плане автотестов;
   • demo/smoke_plan_upload_race.mjs, smoke_save_race.mjs, smoke_ws_resilience.mjs, smoke_version_recovery.mjs — существуют, как названо в плане автотестов п.6.
4. Продуктового кода к задаче ещё нет (этап — ревью ТЗ), поэтому гейты typecheck/test/build/golden не прогонялись и не требовались.

Находки

Medium (в скоупе задачи) — двусмысленная ссылка на тестовый артефакт в AC7 и списке файлов

AC7 и раздел «Ожидаемые файлы и модули» называют test/config-adoption-ownership.test.mjs как один из свидетелей порядка compare → asset readiness → continuity → adopt → reload tail → caller hook. Этот файл уже существует и уже документирован (docs/ARCHITECTURE.md:1375-1393, докстринг файла) как нечто другое: чисто статический regex-сканер исходников, доказывающий не поведенческий порядок операций, а то, что _cfgRev/_layoutRev/fingerprint пишет только ConfigAdoption (ратчет-инвариант #500, счётчики BODY_STAGING_ALLOWLIST могут только уменьшаться). Он не выполняет ни одного _reloadConfigOnly, не мокает prepareImage, не видит async-порядка вообще — механически не способен подтвердить то, что ему приписывает AC7.

Сценарий проявления: разработчик, ориентируясь на прямое упоминание в ТЗ, либо (а) добавляет поведенческие async-тесты заявки/generation в этот файл, ломая его текущую единственную задачу и докстринг («one owner for config/layout identity»), либо (b) недоумевает и оставляет AC7 доказанным только test/config-adoption.test.mjs, а «ownership tests» в ревью кода считается недоказанным пунктом AC. Оба исхода — трение, которого не было бы при точной ссылке.

Это не блокирует ТЗ целиком: основной свидетель AC7 — test/config-adoption.test.mjs, который действительно предназначен для поведенческих проверок (план автотестов п.1, deferred-promise матрица) и способен нести эту нагрузку в одиночку. Также не пострадала ни одна связь с docs/CONFIG-COMPATIBILITY.md или другим каноном — это чисто internal naming/reference вопрос.

Правка: в AC7 и в «Ожидаемых файлах» либо явно развести два разных смысла слова «ownership» (например, назвать новый файл/describe-блок иначе — test/config-adoption-attempt.test.mjs или новый describe в config-adoption.test.mjs), либо прямо указать, что config-adoption-ownership.test.mjs в этой задаче не трогается и упомянут по ошибке. Без правки автор может честно закрыть AC7 текстом «доказано config-adoption.test.mjs», но текущая формулировка ТЗ вводит в заблуждение.

Проверка обязательных разделов (PROCESS.md §7.1)

Все обязательные разделы на месте и содержательны: сценарий · что человек увидит до/после · проблема (аудиторский блок) · скоуп и не-скоуп · контракт поведения (К1–К9) · UX (пусто, обоснованно) · модель данных и миграция (нет изменений схемы, обоснованно) · i18n (нет) · критерии приёмки AC1–AC9 с доказательством (unit/smoke/lifecycle matrix/force/reset/adoption regression/mutation/gates) · план автотестов · риски · откат · release-артефакты.

• Сценарий/видимое поведение — персона-центричны (SCOPE J1/J6), не используют внутренних терминов реализации сверх необходимого; «claim», «generation», «high-water» вводятся только в контракте (К1–К9), не в сценарии.
• AC однозначны и проверяемы, у AC1/AC7/AC8 явно назван отрицательный свидетель («тест умеет падать» — уже в самом ТЗ, а не отложено на код-ревью). AC2/AC5 требуют реального host/lifecycle-прогона, а не последовательного unit-теста — соответствует явному требованию «Ожидаемого результата» из исходного аудиторского блока issue.
• Технические предположения оформлены отдельным блоком «можно менять на ревью», 5 пунктов, каждый — реальное техническое решение (кто выигрывает гонку, что считается новым baseline, поведение superseded, судьба signer-кэша просроченной попытки, исключение layout reload), ни один не маскирует продуктовый вопрос.
• Скоуп/не-скоуп согласован с явным исключением протокола backend/WS, глобальной сериализации reload и правила «никогда не принимать меньшую revision» — совпадает с «Ожидаемым результатом» аудита («не вводить безусловный вечный запрет»).
• Риски — шесть пунктов, каждый называет конкретный способ, которым наивная реализация могла бы сломать defense (не декоративный список).
• Полный трек оправдан явно: названы нарушенные критерии small (multi-client state contract, несколько lifecycle-границ, асинхронный asset gate, риск для continuity/cache) — соответствует §5.

Что не проверялось и почему

• Гейты typecheck/test/build/golden/invariants — не прогонялись: продуктового кода ещё нет, предмет этапа — текст ТЗ.
• Реализация request-owner/coordinator модуля, будущего мутанта AC8 и содержимого demo/smoke_config_reload_race.mjs — не существуют на этом этапе, их проверит код-ревью.
• Частота гонки на реальном HA в проде — сам аудит и ТЗ прямо признают это неизмеренным; для приоритета P2 и полного трека этого достаточно, отдельно не перепроверялось.
• Совместимость с бэкендом/WS-протоколом — вне скоупа задачи по её же формулировке, не проверялась.

Вердикт

Жёлтый: единственная находка — Medium в скоупе задачи (двусмысленная ссылка на test/config-adoption-ownership.test.mjs в AC7 и списке файлов). High нет. Технический контракт (К1–К9), причина по коду и большинство AC подтверждены прямым чтением кода на названном SHA, а не приняты на веру. Автор правит ссылку на тестовый артефакт в теле issue, изменение проходит повторный цикл ревью ТЗ по дельте (PROCESS.md §2.10).

---

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

• Ветка: dev, коммит 6d2facc2e377 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: bd3acd278b0c6dd3d0cc572096f8c0d2c7e3b38c

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

• Тело issue: 06cae6bc0c1c834e055a858a18d406231ff7a696f929be70277a82a675f6779a
• Вердикт конвейера: yellow · High 0