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

type, work_item_id, verdict, version

type	work_item_id	verdict	version
review	ORCH-017	REQUEST_CHANGES	3

Review ORCH-017

Summary

Основная фича (прямые BRD-/Plane-ссылки в notify_approve_requested) реализована качественно и соответствует ТЗ, 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.

Соответствие ТЗ

• §3.1–§3.2, §4–§6 (фича уведомления) — выполнено. BRD-/Plane-ссылки строятся независимо (_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) при неполноте ТЗ — возврат на Анализ, а не тихое расширение объёма. ✘

Соответствие 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. ✘

Качество кода

Сама правка _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).

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 утверждает обратное.

P2 — Should fix

• 13-test-report.md устарел относительно правки гейта: отчёт покрывает только нотификационные TC и фиксирует «434 passed» из прогона, предшествующего e62d51a; новые кейсы tests/test_qg.py и сама правка гейта в отчёте не отражены. (Канонический ре-тест — на стадии testing после устранения P0/P1.)

Документация

Правило «изменён src/ → обновлена документация в том же PR» по фиче уведомления — выполнено (CHANGELOG.md, .env.example, docs/operations/INFRA.md, ADR-001). ✔

Для правки гейта документация неконсистентна: CHANGELOG (Fixed) и docs/architecture/README.md обновлены, НО ADR-001 Р-5 и ТЗ §7 продолжают утверждать, что QG/машинные вердикты не меняются — прямое противоречие коду того же PR. Требуется: либо вынос правки в отдельный work item с собственным ADR, либо (при одобрении Owner) приведение ADR-001/ТЗ §7 в соответствие и отдельный ADR на изменение семантики разделяемого testing-гейта.