houseplan-card/docs/reviews/SPEC-REVIEW-43-r5.md

SPEC-REVIEW-43-r5

• Issue: #43 — «Диалог помощи и обратной связи с обезличенным support report»
• Этап: ТЗ на ревью (PROCESS.md §2.4), полный трек (issue не помечен small)
• Заход: r5 · блокирующих циклов израсходовано 3 из 4 до этого раунда
• Материал: docs/specs/043-private-support-report.md, scripts/support-relay/README.md, scripts/support-relay/tests/test_relay.py на ветке issue/43-help-feedback
• SHA материала r4 (названо в самом вердикте r4): cd8d01ac
• SHA материала этого раунда (git rev-parse HEAD непосредственно перед выводом): 3ce5bc0ea2bab55c7c586d63b78a76459fd5c4d2
• Предыдущий документ: docs/reviews/SPEC-REVIEW-43-r4.md

Скоуп разбора

Раунд r4 закончился жёлтым вердиктом с единственной находкой уровня Medium (в скоупе, техническая): §9.2/AC12a/§14.4 описывали чтение X-Forwarded-For как безусловный факт, хотя в коде это работает только под флагом HP_RELAY_TRUSTED_PROXY (default on), а ветка trusted_proxy=False не имела покрытия тестами.

Дельта cd8d01ac..3ce5bc0e строго докс- и тест-only:

docs/reviews/SPEC-REVIEW-43-r4.md         | 210 +++++++++++++
docs/specs/043-private-support-report.md  |  21 +-
scripts/support-relay/README.md           |  16 +-
scripts/support-relay/tests/test_relay.py |  51 ++++

(первый файл — публикация артефакта r4 предыдущим прогоном конвейера, не предмет этого разбора). Кода продукта (src/**, custom_components/**) дельта не касается, новая подсистема не задета, объём несравним с задачей — это классический случай «предмет раунда — дельта», разбор по ней, а не заново.

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

1. Прочитан полный текст docs/specs/043-private-support-report.md заново (не только изменённые абзацы) — чтобы дельта на 21 строку не спрятала контекстную рассинхронизацию с остальным документом.
2. Сверено текстовое утверждение §9.2 с фактическим кодом: scripts/support-relay/hp_relay/app.py:147-158 (_source()) и scripts/support-relay/hp_relay/config.py:82 (trusted_proxy default) — поведение обеих позиций переключателя дословно совпадает с новым текстом ТЗ.
3. Прогнан полный набор relay: cd scripts/support-relay && python3 -m unittest discover -s tests -q → 36 проверок, все зелёные.
4. Дисциплина «тест умеет падать» применена к новому тесту test_direct_node_ignores_the_forwarded_header лично, а не со слов автора: в hp_relay/app.py::_source() заменено if service.cfg.trusted_proxy: на if True: (эмуляция мутации «переключатель проигнорирован») — python3 -m unittest tests.test_relay.DirectNodeTestCase -v упал: AssertionError: 3 != 1 (три разных ключа источника вместо одного). Файл восстановлен из копии, git diff --stat hp_relay/app.py после отката — пусто, повторный полный прогон — снова 36/36.
5. Проверено, что существующий тест test_client_cannot_pick_its_own_rate_bucket (позиция переключателя по умолчанию, trusted_proxy=True) не тронут и продолжает покрывать вторую половину AC12a.
6. node scripts/process-gate.mjs — офлайн-прогон на HEAD, диапазон origin/dev..HEAD, 12 коммитов: «гейт пройден, предупреждений 0».
7. Проверены трейлеры обоих коммитов дельты (git log --format='%B' cd8d01ac..HEAD): у обоих Issue: #43 и User-Visible: no — верно, дельта не меняет пользовательское поведение.
8. Прочитана вся история issue #43 (18 комментариев) для контекста: подтверждено, что все пять внешних DoR-зависимостей §17 закрыты предыдущими комментариями владельца (deployment target, канал доставки/credentials, 30-дневное удаление, staging для CI, ответственный), последний — «blocked можно снимать: внешних зависимостей у реализации больше нет» (2026-09-01 22:04). Это не переоценивается заново в этом раунде — оно не относится к предмету дельты (флаг доверенного прокси) и не изменилось между r4 и r5.

Закрытие раунда r4

Находка r4	Чем закрыта	Где видно
Medium. §9.2 описывает чтение X-Forwarded-For как безусловный факт, хотя в коде это работает только под флагом HP_RELAY_TRUSTED_PROXY (default on); AC12a и §14.4 не называют флаг; нет теста на позицию trusted_proxy=False.	§9.2 переписан: флаг назван по имени, обе позиции описаны явно («включён» / «выключен»), добавлено требование «за прокси выключать запрещено» с обоснованием цены (весь публичный эндпоинт делил бы одну корзину). AC12a переписан под обе позиции с указанием двух тестов. §14.4 получил строку про мутацию «читать заголовок независимо от переключателя». Написан и подтверждён тест test_direct_node_ignores_the_forwarded_header (не только обещан в тексте — прогнан и лично провален мутацией).	docs/specs/043-private-support-report.md §9.2 (абзац «Behind a reverse proxy…»), §13 AC12a, §14.4; scripts/support-relay/tests/test_relay.py:312-361 (DirectNodeTestCase); код без изменений — hp_relay/app.py:147-158 уже соответствовал описанию.

Закрыто полностью, без остатка: расхождение между текстом ТЗ и кодом, из-за которого требование «не пережило бы рефакторинг app.py» (как выразился автор в комментарии к r4), теперь закреплено и текстом, и работающим тестом, способным упасть.

Унаследовано из r4 (и через r4 из более ранних раундов)

Со ссылкой на docs/reviews/SPEC-REVIEW-43-r4.md, SHA cd8d01ac, без повторной проверки в этом раунде — дельта их не касается:

• §1–8 (сценарий, персона, скоуп, решения владельца, UX-контракт кнопки/диалога/ формы/preview/submit) — полностью проверены в r1 построчно против кода (src/houseplan-card.ts, src/houseplan-editor-runtime.ts), включая соответствие пяти утверждений §3 фактическому состоянию;
• §7 (envelope, allowlist, псевдонимизация, privacy invariant, лимиты) — проверен в r1 на внутреннюю непротиворечивость allowlist/exclusion списков;
• §8 (backend API preview/submit) — контракт «просмотренные байты = отправленные байты» (кешированный TTL-токен + mutation-gate) проверен в r1 как спроектированный в контракт, а не оставленный реализации;
• §9.1, §9.3 (архитектура relay, каналы доставки Telegram/ha_webhook, ретеншн) — полностью разобраны заново в r3 после смены продакшн-канала на боевом стенде (это была не локальная правка, а смена контракта поведения, разбор шёл целиком, не по дельте); r4 унаследовал их без изменений, этот раунд — тоже, дельта их не трогает;
• §10–13 (кроме AC12a), §15–20 (state/compat, touch/a11y §11 с канонической фразой «Touch editor: supported», i18n, AC1–AC17 кроме AC12a, test plan §14.1–14.3, performance-бюджеты, риски, §17 DoR-зависимости, §18 release-артефакты, §19 rollback, §20 assumed-блок) — проверены в r1–r3, последняя правка (r4) их не касалась, эта (r5) — тоже;
• классификация scripts/support-relay/** как класс B (AGENTS.md/PROCESS.md, scripts/process-gate.mjs::classify() regex /^scripts\//) — закрыта в r2 чтением regex и живым прогоном гейта, не пересматривалась;
• фактическое закрытие всех пяти пунктов §17 DoR (deployment target, канал доставки, 30-дневный ретеншн, staging, ответственный) — установлено серией комментариев владельца в issue между r2 и r3 с воспроизведёнными командами (curl .../health, systemd-таймеры, README) и последним подтверждением «внешних зависимостей больше нет»; это не предмет спек-ревью ТЗ как текста (ТЗ корректно описывает эти зависимости как внешние в §17), а операционный факт вне дельты — принят как есть.

Находки

Low, снимается решением ревьюера с записью, цикл не расходует. scripts/support-relay/README.md (раздел «Тесты»): текст утверждает «четырнадцать мутаций рабочего кода» и затем перечисляет их в скобках — пересчёт даёт тринадцать пунктов (снять сверку хеша · разрешить лишнюю часть · не чистить управляющие символы · снять лимит · писать адрес в журнал · игнорировать идемпотентность · отключить рубильник · отключить ретеншн · не проверять секции пакета · брать первый элемент X-Forwarded-For · подменить source в вебхуке · приложить пакет к вебхуку · читать заголовок независимо от переключателя — 13). Расхождение уже было в r4-версии текста (там «тринадцать» против фактических двенадцати пунктов списка) и в этом раунде перенесено на единицу дальше вместе с добавлением тринадцатого пункта. Не блокирует: это описательное число в README для человека, читающего перед раскруткой relay, оно не служит доказательством ни одного AC (доказательство — сам прогон unittest discover, который лично прогнан и даёт 36/36) и не является тем «одно число — один источник» пользовательским значением, о котором предупреждает §8 PROCESS.md — это внутренний devops-README, а не пользовательский интерфейс. Снимаю без возврата автору; при следующей правке этого README посчитать пункты заново.

Других находок нет: High — 0, Medium — 0.

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

• Текст §9.2/AC12a/§14.4 после правки дословно соответствует поведению кода (_source() в app.py, default в config.py) для обеих позиций переключателя;
• Новый тест DirectNodeTestCase.test_direct_node_ignores_the_forwarded_header реально существует, реально прогоняется, реально падает на релевантной мутации (проверено лично, не со слов автора) и не задевает остальные 35 проверок;
• Существующий тест на позицию trusted_proxy=True (test_client_cannot_pick_its_own_rate_bucket) не тронут дельтой и по-прежнему проходит;
• Деплой-артефакты (scripts/support-relay/deploy/env.example, Caddyfile.fragment) уже фиксируют HP_RELAY_TRUSTED_PROXY=1 и перезапись заголовка на обоих продовых хостах — согласуется с новым текстом §9.2 «Both production hosts run behind Caddy, so both keep the switch on»;
• node scripts/process-gate.mjs зелёный (12 коммитов в диапазоне, 0 предупреждений); трейлеры обоих коммитов дельты корректны (Issue: #43, User-Visible: no);
• Документ ТЗ остаётся внутренне непротиворечивым за пределами дельты — не найдено новых противоречий между §9.2 и остальными разделами при повторном чтении документа целиком.

Чего не проверял и почему

• npx tsc --noEmit, npm test, npm run build со сверкой копий бандла — дельта не трогает src/**/custom_components/**; докс- и Python-тест-онли изменение не имеет для них предмета. (Ранее на цепочке этого issue Validate на предыдущих relay-коммитах уже проходил зелёным на CI — здесь новых фронтенд/бэкенд-в-смысле-Python-интеграции изменений нет вовсе.)
• node scripts/check-docs.mjs — требуется только при изменении src/**; здесь его нет.
• node scripts/model-invariants.mjs — геометрия/layout/ссылки на неё не затронуты; речь только о rate-limit relay.
• python -m pytest tests_backend -q — эта команда покрывает custom_components/houseplan/**, не relay-поддерево; relay использует отдельный unittest discover -s scripts/support-relay/tests, который прогнан лично (см. выше).
• Браузерные смоки, golden:verify, performance-профили — нет UI/визуальных изменений в этой дельте; продуктовый код (src/**) для #43 ещё не начат (подтверждено комментарием автора «Продуктовый код (src/**) не начат — §17 соблюдён» и отсутствием изменений src/** во всём диапазоне origin/dev..HEAD).
• Повторная проверка §9.1/§9.3 (архитектура каналов доставки, ретеншн) и внешних DoR-фактов §17 «с нуля» (например, повторный curl на support.houseplan.tech/support-staging.houseplan.tech) — не предмет этой дельты; принято по унаследованной цепочке r2→r3 с воспроизведёнными в issue командами, см. раздел «Унаследовано из r4» выше.

Вердикт

Единственная Medium-находка r4 закрыта предметно (текст ТЗ + код, который уже был правильным, + новый работающий и лично провалившийся на мутации тест). Единственная новая находка этого раунда — Low, косметическая, в devops-README, не влияющая ни на один AC, снята с записью без возврата автору.

Зелёный. ТЗ готово к S5-ready. С учётом комментариев в issue все пять внешних DoR-зависимостей §17 отмечены автором как закрытые — если это подтверждается конвейером/владельцем, blocked может сниматься вместе с переходом статуса.