reviewer(ET): auto-commit from reviewer run_id=120
This commit is contained in:
@@ -2,47 +2,56 @@
|
||||
type: review
|
||||
work_item_id: ORCH-017
|
||||
verdict: APPROVED
|
||||
version: 1
|
||||
version: 2
|
||||
---
|
||||
|
||||
# Review ORCH-017
|
||||
|
||||
## Summary
|
||||
PR добавляет в пингующее уведомление об апруве BRD (`notify_approve_requested`) две
|
||||
кликабельные HTML-ссылки — на `01-brd.md` в Gitea (branch-view) и на issue в Plane.
|
||||
Реализация полностью соответствует ТЗ (`02-trz.md`), всем 10 критериям приёмки
|
||||
(`03-acceptance-criteria.md`) и принятому ADR-001. Все 434 теста зелёные, документация
|
||||
обновлена в этом же PR. Блокеров и must-fix findings нет.
|
||||
PR добавляет в пингующее уведомление об апруве BRD (`notify_approve_requested`)
|
||||
две кликабельные HTML-`<a>`-ссылки — на `01-brd.md` в Gitea (branch-view) и на
|
||||
issue в Plane. Реализация затрагивает только `src/config.py` и
|
||||
`src/notifications.py`. Независимая проверка подтверждает соответствие ТЗ
|
||||
(`02-trz.md`), всем 10 критериям приёмки (`03-acceptance-criteria.md`) и
|
||||
принятому ADR-001. Полный прогон `pytest tests/ -q` → **434 passed**, целевые
|
||||
тесты (`test_notify_approve_links.py`, `test_analysis_approve_flow_links.py`,
|
||||
`test_telegram_tracker.py`, `test_notify_done_regression.py`) → 43 passed.
|
||||
Документация обновлена в том же PR. Блокеров и must-fix findings нет.
|
||||
|
||||
## Соответствие ТЗ и AC
|
||||
- **AC-1 / AC-2** — BRD- и Plane-ссылки строятся (`_build_brd_link`, `_build_plane_issue_link`),
|
||||
встроены в текст того же сообщения (TC-01/TC-02). ✔
|
||||
- **AC-3** — фолбэки `gitea_public_url→gitea_url` и `plane_web_url→plane_api_url` соблюдены (TC-03). ✔
|
||||
- **AC-4** — призыв «Переведите задачу в статус Approved …» сохранён (TC-04). ✔
|
||||
- **AC-5** — ровно одно пингующее (не silent) сообщение; `send_telegram` не дублируется,
|
||||
трекер обновляется как раньше (TC-05). ✔
|
||||
- **AC-6** — graceful degradation: SELECT полей в try/except, каждая ссылка строится
|
||||
независимо и опускается при нехватке данных; loopback-guard выкидывает localhost-URL
|
||||
(TC-06/07/07b). Исключения не пробрасываются. ✔
|
||||
- **AC-7** — `parse_mode=HTML` сохранён, динамические подписи через `html.escape`,
|
||||
корректная `<a>`-разметка (TC-08/08b). ✔
|
||||
- **AC-8** — логика апрува не тронута: в `src/` изменены только `config.py` и
|
||||
`notifications.py`; `qg/checks.py`, `stages.py`, `:approved:`-handler без изменений. ✔
|
||||
- **AC-10** — `pytest tests/ -q` → 434 passed. ✔
|
||||
## Соответствие ТЗ
|
||||
- §3.1 — BRD- и Plane-ссылки строятся независимо (`_build_brd_link`,
|
||||
`_build_plane_issue_link`), встроены в текст того же одного сообщения; призыв
|
||||
«Переведите задачу в статус Approved …» сохранён; `html.escape` на динамике. ✔
|
||||
- §3.1 порядок действий — `mark_brd_review_started → update_task_tracker →
|
||||
send_telegram(msg)` (пингующий, не silent) сохранён, проверено по коду. ✔
|
||||
- §3.2 — `Settings.plane_web_url` (env `ORCH_PLANE_WEB_URL`); фолбэк
|
||||
`plane_web_url → plane_api_url`. ✔
|
||||
- §4/§5/§7 — API, схема БД и реестр `QG_CHECKS` не затронуты (подтверждено
|
||||
diff: в `src/` только `config.py` и `notifications.py`). ✔
|
||||
|
||||
## Соответствие ADR
|
||||
- **Р-1** — HTML-ссылки в тексте, сигнатура `send_telegram` НЕ изменена (inline-кнопки
|
||||
отклонены) — blast radius минимальный, безопасно для self-hosting. ✔
|
||||
- **Р-2** — полный путь Plane по uuid `{web_base}/{workspace}/projects/{project_id}/issues/{issue_id}/`. ✔
|
||||
- **Р-3** — новая настройка `ORCH_PLANE_WEB_URL` + loopback-guard (`_is_loopback_base`,
|
||||
набор `localhost/127.0.0.1/0.0.0.0/::1`, пустой URL = unusable). ✔
|
||||
- **Р-4 / Р-5** — graceful degradation как контракт; порядок действий
|
||||
`mark_brd_review_started → update_task_tracker → send_telegram` сохранён. ✔
|
||||
## Соответствие ADR-001
|
||||
- Р-1 — HTML-ссылки в тексте; сигнатура `send_telegram` НЕ изменена
|
||||
(inline-кнопки отклонены) → минимальный blast radius, безопасно для
|
||||
self-hosting. ✔
|
||||
- Р-2 — полный путь Plane по uuid
|
||||
`{web_base}/{workspace}/projects/{project_id}/issues/{plane_issue_id}/`. ✔
|
||||
- Р-3 — `ORCH_PLANE_WEB_URL` + loopback-guard (`_is_loopback_base`:
|
||||
`localhost/127.0.0.1/0.0.0.0/::1`, пустой/битый URL = unusable → ссылка
|
||||
опускается). ✔
|
||||
- Р-4/Р-5 — graceful degradation как контракт построения ссылок (SELECT в
|
||||
try/except, каждая ссылка независима); инвариант «одно сообщение, без дублей»
|
||||
соблюдён. ✔
|
||||
|
||||
## Качество кода
|
||||
- Публичные/вспомогательные функции снабжены содержательными docstrings со ссылками на AC/ADR.
|
||||
- Защитные try/except вокруг I/O (БД, projects), логирование без падения потока.
|
||||
- Тесты содержательные, замаплены на TC-01…TC-08 из `04-test-plan.yaml`, сеть и БД изолированы.
|
||||
- Вспомогательные функции снабжены содержательными docstrings со ссылками на
|
||||
AC/ADR.
|
||||
- Все обращения к БД и `get_project_by_repo` обёрнуты в try/except с
|
||||
логированием; поток уведомлений не падает при нехватке данных (AC-6).
|
||||
- Тесты содержательные, замаплены на TC из `04-test-plan.yaml`, изолируют сеть
|
||||
и БД.
|
||||
- Проверено наличие зависимостей: `import html`, `_get_settings`, поля Settings
|
||||
`gitea_public_url/gitea_owner/gitea_url`, `plane_workspace_slug/plane_web_url/
|
||||
plane_api_url`.
|
||||
|
||||
## Findings
|
||||
|
||||
@@ -56,17 +65,17 @@ PR добавляет в пингующее уведомление об апру
|
||||
- нет
|
||||
|
||||
### P3 — Nice-to-have
|
||||
- [ ] `docs/architecture/README.md` не обновлён — но и не требует обновления: API,
|
||||
стадии, реестр QG и компоненты не менялись (правка — отображение, не управление).
|
||||
Env-карта обновлена в `docs/operations/INFRA.md`, что корректно. Замечание косметическое.
|
||||
- `docs/architecture/README.md` не обновлён, и это корректно: API, стадии,
|
||||
реестр QG и компоненты не менялись; trigger-файлы из футера README
|
||||
(`stages.py`, `qg/checks.py`, `main.py`) не затронуты. Env-карта обновлена в
|
||||
`docs/operations/INFRA.md`. Замечание чисто информационное.
|
||||
|
||||
## Документация
|
||||
Обновлена корректно и в этом же PR:
|
||||
- `CHANGELOG.md` — запись о фиче (раздел Unreleased/Added). ✔
|
||||
- `.env.example` — новая `ORCH_PLANE_WEB_URL` с описанием фолбэка и loopback-семантики. ✔
|
||||
Правило «изменён `src/` → обновлена документация в том же PR» выполнено:
|
||||
- `CHANGELOG.md` — запись о фиче (Unreleased/Added). ✔
|
||||
- `.env.example` — новая `ORCH_PLANE_WEB_URL` с описанием фолбэка и
|
||||
loopback-семантики. ✔
|
||||
- `docs/operations/INFRA.md` — env-карта дополнена `ORCH_PLANE_WEB_URL`. ✔
|
||||
- `src/config.py` — поле документировано inline-комментарием. ✔
|
||||
- ADR `docs/work-items/ORCH-017/06-adr/ADR-001-telegram-approve-links.md` — фиксирует выбор
|
||||
формата (HTML vs кнопки) и формат Plane-URL. ✔
|
||||
|
||||
Требование «изменён src/ → обновлена документация» выполнено.
|
||||
- ADR `docs/work-items/ORCH-017/06-adr/ADR-001-telegram-approve-links.md` —
|
||||
фиксирует выбор формата (HTML vs кнопки) и формат Plane-URL. ✔
|
||||
|
||||
Reference in New Issue
Block a user