reviewer(ET): auto-commit from reviewer run_id=124

docs/work-items/ORCH-017/12-review.md

@@ -2,99 +2,82 @@
 type: review
 work_item_id: ORCH-017
 verdict: REQUEST_CHANGES
-version: 3
+version: 4
 ---
 
 # Review ORCH-017
 
 ## Summary
 Основная фича (прямые BRD-/Plane-ссылки в `notify_approve_requested`) реализована
-качественно и соответствует ТЗ, ADR-001 и всем критериям приёмки — это было
-независимо подтверждено в review v2 (изменения только в `src/config.py` и
-`src/notifications.py`).
+качественно и соответствует ТЗ, ADR-001 и всем критериям приёмки (подтверждено в
+review v2: изменения по фиче — только `src/config.py` и `src/notifications.py`).
 
-Однако **после** review v2 в этот же work item внесён коммит `e62d51a`
-`fix(qg): testing gate reads documented tester result: frontmatter key (ORCH-017)`,
-который меняет `src/qg/checks.py` — функцию `_parse_tests_verdict` **разделяемого**
-гейта `check_tests_passed`. Это правка машинного вердикта / управления конвейером,
-затрагивающая ВСЕ проекты из общего прод-инстанса (self-hosting blast radius), и она
-**прямо противоречит** собственным управляющим документам задачи (ТЗ §7 и ADR-001 Р-5),
-которые фиксируют, что QG_CHECKS и машинные вердикты НЕ меняются. Изменение не
-оформлено отдельным ADR и не проведено через возврат на Анализ. → REQUEST_CHANGES.
+P0 из review v3 (правка разделяемого гейта `check_tests_passed` коммитом `e62d51a`,
+нарушавшая ADR-001 Р-5 и ТЗ §7) **снят**: коммит `d615747` откатил изменение
+`src/qg/checks.py` (вынесено в отдельный work item ORCH-47 со своим ADR). Код гейта
+теперь идентичен `main` (читает только `verdict:`/`status:`); ADR-001 Р-5 и ТЗ §7
+снова консистентны с кодом. ✔
+
+Однако откат кода **не сопровождён откатом документации**: `CHANGELOG.md` и
+`docs/architecture/README.md` всё ещё описывают откаченную правку гейта и ссылаются
+на не существующие в этом PR тесты `tests/test_qg.py`. Это новый doc↔code конфликт
+(golden source). → REQUEST_CHANGES (P1).
 
 ## Соответствие ТЗ
-- §3.1–§3.2, §4–§6 (фича уведомления) — выполнено. BRD-/Plane-ссылки строятся
-  независимо (`_build_brd_link`, `_build_plane_issue_link`), встроены в текст
-  одного сообщения; призыв «Переведите задачу в статус Approved …» сохранён;
+- §3.1–§3.2, §4–§6 (фича уведомления) — выполнено. `_build_brd_link` /
+  `_build_plane_issue_link` строят ссылки независимо, встроены в текст одного
+  сообщения; призыв «Переведите задачу в статус Approved …» сохранён;
   `html.escape` на динамике; порядок `mark_brd_review_started → update_task_tracker
   → send_telegram(msg)` соблюдён; `Settings.plane_web_url` + фолбэк добавлены. ✔
-- **§7 — НАРУШЕНО.** ТЗ §7 дословно: «Требования к новым QG checks: Нет. Реестр
-  `QG_CHECKS`, стадии и **машинные вердикты не меняются**». Коммит `e62d51a`
-  меняет машинный вердикт гейта `check_tests_passed` (читает теперь `result:`).
-  По канону (CLAUDE.md правило 4) при неполноте ТЗ — возврат на Анализ, а не
-  тихое расширение объёма. ✘
+- §7 — соблюдено. Реестр `QG_CHECKS`, стадии и машинные вердикты в коде не меняются
+  (правка гейта откачена в `d615747`). ✔
 
 ## Соответствие ADR
-- ADR-001 (Р-1…Р-4) по фиче уведомления — соблюдён. ✔
-- **ADR-001 Р-5 — НАРУШЕНО.** Р-5 фиксирует: «Реестр `QG_CHECKS`, стадии,
-  `:approved:`-handler, `check_analysis_approved` — без изменений (правка —
-  отображение, **не управление конвейером**)». Коммит `e62d51a` меняет именно
-  управление (логику прохождения testing-гейта). ADR-001 теперь содержит
-  утверждение, противоречащее коду того же PR → golden-source конфликт. ✘
-- **Нет ADR на изменение семантики гейта.** Правка разделяемого machine-verdict
-  гейта, влияющая на все проекты, — архитектурное решение и требует ADR
-  (CLAUDE.md правило 2). Заведён только ADR-001 (про ссылки); ADR на правку
-  `check_tests_passed` отсутствует (`docs/architecture/adr/` и `06-adr/` его не
-  содержат). README/CHANGELOG-строк недостаточно — они не снимают противоречие
-  с ADR-001 Р-5. ✘
+- ADR-001 (Р-1…Р-5) — соблюдён. Ссылки HTML-`<a>` в тексте, `send_telegram` не
+  тронута; полный Plane-URL по uuid; `ORCH_PLANE_WEB_URL` + loopback-guard
+  (`_is_loopback_base`); graceful degradation; «одно сообщение, без дублей». ✔
+- ADR-001 Р-5 vs код — конфликт снят откатом гейта. ✔
 
 ## Качество кода
-Сама правка `_parse_tests_verdict` технически корректна и защитна: `result:`
-добавлен как равноправное поле наряду с `verdict:`/`status:`, отрицательный токен
-в любом поле остаётся авторитетным (ET-013 кейс сохранён), добавлены адресные
-тесты (`test_result_pass_only_passes`, `test_result_fail_only_fails`). Претензий к
-самой реализации нет — проблема не в коде, а в нарушении объёма/ADR и self-hosting
-governance: изменение разделяемой прод-инфраструктуры протащено внутрь задачи про
-текст уведомления вместо отдельного work item с собственным ADR.
-
-Фича уведомления (`notifications.py`/`config.py`) — без замечаний (см. review v2).
+Фича `notifications.py`/`config.py` — без замечаний. Чтение полей задачи
+(`_get_task_link_fields`) и обе сборки ссылок защищены try/except и никогда не
+роняют alert (AC-6); loopback-guard корректно опускает неклика­бельный Plane-URL
+(AC-2/AC-3); `html.escape(..., quote=True)` на href и `html.escape(work_item_id)`
+на подписи (AC-7). Тесты `tests/test_notify_approve_links.py`,
+`tests/test_analysis_approve_flow_links.py` присутствуют и содержательны.
 
 ## Findings
 
 ### P0 — Blocker
-- [ ] **Нарушен ADR-001 Р-5 и ТЗ §7.** Коммит `e62d51a` меняет машинный вердикт
-      разделяемого гейта `check_tests_passed` (`src/qg/checks.py::_parse_tests_verdict`),
-      тогда как ADR-001 Р-5 и ТЗ §7 явно фиксируют неизменность `QG_CHECKS` и
-      машинных вердиктов («правка — отображение, не управление конвейером»). Код
-      PR противоречит собственным управляющим артефактам задачи.
-      **Резолюция (одна из):**
-      (а) вынести правку гейта в отдельный work item со своим ADR (канон:
-          ТЗ неполно → возврат на Анализ, не scope-creep); либо
-      (б) при явном одобрении Owner на сохранение правки в ORCH-017 — привести
-          ADR-001/ТЗ §7 в соответствие с кодом и завести отдельный ADR,
-          документирующий изменение семантики разделяемого testing-гейта и его
-          self-hosting blast radius.
+- (нет)
 
 ### P1 — Must fix
-- [ ] **Нет ADR на архитектурное изменение разделяемого гейта** (CLAUDE.md правило 2).
-      Изменение чтения машинного вердикта `check_tests_passed` влияет на все проекты
-      общего прод-инстанса (enduro-trails и др.) — это архитектурное решение,
-      требующее ADR. Сейчас задокументировано лишь строкой README + CHANGELOG +
-      комментарием в коде; ADR отсутствует, а ADR-001 утверждает обратное.
+- [ ] **Документация описывает откаченный код (doc↔code конфликт).** После
+      revert-коммита `d615747` код `src/qg/checks.py` НЕ читает `result:` (только
+      `verdict:`/`status:`), но документация осталась от состояния `e62d51a`:
+      - `docs/architecture/README.md:61` утверждает, что `check_tests_passed`
+        читает `verdict:`/`status:`/`result:` — это ложно для текущего кода и
+        вводит в заблуждение по поведению разделяемого прод-гейта (self-hosting:
+        tester, написавший только `result: PASS`, реально провалит гейт).
+      - `CHANGELOG.md:24` (секция Fixed) содержит запись о правке гейта
+        `check_tests_passed` под тегом ORCH-017 и ссылается на отсутствующие в PR
+        тесты `tests/test_qg.py::TestCheckTestsPassed::test_result_pass_only_passes`
+        / `…::test_result_fail_only_fails`.
+      **Резолюция:** убрать из ORCH-017 PR обе записи (откатить README:61 к
+      формулировке `main` и удалить CHANGELOG-entry про гейт) — правка гейта
+      принадлежит ORCH-47 и должна документироваться там вместе с её кодом.
 
 ### P2 — Should fix
-- [ ] `13-test-report.md` устарел относительно правки гейта: отчёт покрывает только
-      нотификационные TC и фиксирует «434 passed» из прогона, предшествующего
-      `e62d51a`; новые кейсы `tests/test_qg.py` и сама правка гейта в отчёте не
-      отражены. (Канонический ре-тест — на стадии testing после устранения P0/P1.)
+- [ ] `13-test-report.md` (`result: PASS`) относится к прогону, включавшему
+      откаченную правку гейта; после устранения P1 канонический ре-тест — на
+      стадии testing (отчёт не должен ссылаться на снятые из PR изменения).
 
 ## Документация
 Правило «изменён `src/` → обновлена документация в том же PR» по фиче уведомления —
-выполнено (`CHANGELOG.md`, `.env.example`, `docs/operations/INFRA.md`, ADR-001). ✔
+выполнено: `CHANGELOG.md` (Added), `.env.example` (`ORCH_PLANE_WEB_URL`),
+`docs/operations/INFRA.md` (env-карта), ADR-001. ✔
 
-Для правки гейта документация **неконсистентна**: CHANGELOG (Fixed) и
-`docs/architecture/README.md` обновлены, НО ADR-001 Р-5 и ТЗ §7 продолжают
-утверждать, что QG/машинные вердикты не меняются — прямое противоречие коду того же
-PR. Требуется: либо вынос правки в отдельный work item с собственным ADR, либо
-(при одобрении Owner) приведение ADR-001/ТЗ §7 в соответствие и отдельный ADR на
-изменение семантики разделяемого testing-гейта.
+Неконсистентность (P1): документация про откаченную правку гейта `check_tests_passed`
+осталась в `CHANGELOG.md` (Fixed) и `docs/architecture/README.md`, хотя
+соответствующий код отозван (`d615747`) и перенесён в ORCH-47. Доку нужно привести в
+соответствие с кодом этого PR.