docs: review document for #43

Issue: #43
User-Visible: no

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

@@ -0,0 +1,197 @@
+# 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` может сниматься вместе с
+переходом статуса.