From e45f4df2854e6887a5c8793139cb20def581ee42 Mon Sep 17 00:00:00 2001 From: claude-bot Date: Mon, 8 Jun 2026 10:29:27 +0000 Subject: [PATCH] reviewer(ET): auto-commit from reviewer run_id=366 --- docs/work-items/ORCH-067/12-review.md | 90 ++++++++++++++++----------- 1 file changed, 55 insertions(+), 35 deletions(-) diff --git a/docs/work-items/ORCH-067/12-review.md b/docs/work-items/ORCH-067/12-review.md index 50e3c1d..bb470fe 100644 --- a/docs/work-items/ORCH-067/12-review.md +++ b/docs/work-items/ORCH-067/12-review.md @@ -1,58 +1,78 @@ --- type: review work_item_id: ORCH-067 -verdict: REQUEST_CHANGES -version: 1 +verdict: APPROVED +version: 2 --- # Review ORCH-067 ## Summary -Реализация качественная и по существу соответствует ТЗ (`02-trz.md`) и ADR-001: -оффлайн-ядро `plane_status_label` + best-effort live-overlay `_live_plane_branch_override` -(kill-switch / TTL-кэш / короткий таймаут), единый `_plane_issue_url` с переиспользованием -guard'ов ORCH-017, хелперы `plane_issue_link` / `link_for`, дефолт `tracker_mode → bump`, -новые config-флаги. Контракт «never raises» выдержан, схема БД / транспорт -`send_telegram`/`edit_telegram`/`delete_telegram` / `disable_notification` / `STAGE_TRANSITIONS` -/ QG — не тронуты (AC-15 ✓). `pytest tests/ -q` — **907 passed** (AC-16, AC-17 ✓). -Точки §3.3 применены корректно: `merge_gate.py`/`job_reaper.py`/`main.py`/`launcher._notify_failed` -оставлены без ссылки осознанно — в их тексте НЕТ номера задачи (правило «только где упоминается -work_item_id» соблюдено). +Повторное ревью после фикса документации (коммит `7a88f39`). Реализация полностью +соответствует ТЗ (`02-trz.md`), ADR-001 и всем acceptance criteria (`03-acceptance-criteria.md`). -**Блокер — неполная документация.** ТЗ §5 и AC-18 явно требуют обновления `CHANGELOG.md` и -`CLAUDE.md` в том же PR; оба не обновлены. Это нарушает правило «документация = golden source» -(CLAUDE.md, п.6 правил агентов) → REQUEST_CHANGES. +**Код** (`src/notifications.py` — ядро): +- **Req 1 (bump):** дефолт `tracker_mode` сменён `edit → bump` (`src/config.py`); логика + `update_task_tracker`, транспорт `send/edit/delete_telegram`, `disable_notification` и + инвариант «одна карточка на задачу» не тронуты (AC-1..AC-4, AC-15 ✓). +- **Req 2 (статус-строка):** чистый never-raise `plane_status_label(task_row)` (offline-ядро: + stage→статус + `⏸️ In Review` из brd-clock + `⏸️ Awaiting Deploy`, всё без сети) + + best-effort `_live_plane_branch_override` для ветвей, неотличимых offline (Needs Input / + Blocked / Rejected / Cancelled / Deploying / Monitoring). Kill-switch + (`tracker_live_status`), per-issue TTL-кэш (`_LIVE_STATE_CACHE`), короткий таймаут + (`fetch_issue_state(..., timeout=)`, дефолт 10 сохранён → нет регресса reconciler). + Anti-false-positive guard для enduro (`_LIVE_BRANCH_BASE`: deploying/monitoring override + только при отдельном UUID). Прецеденс In Review > overlay соблюдён. `_card_status_label` + обёрнут в try/except → рендер никогда не падает (AC-5..AC-9 ✓). +- **Req 3+4 (кликабельный номер):** единый `_plane_issue_url` устраняет дублирование + резолва проекта/loopback-guard (ORCH-017); `plane_issue_link` (текст=номер) и + `_build_plane_issue_link` (текст=«✅ Задача в Plane») оба зовут его. `link_for` fail-safe + достаёт `repo`/`plane_issue_id` из БД. Применено в заголовке карточки и во ВСЕХ точках + §3.3 с номером задачи (AC-10..AC-14 ✓). + +**Точки §3.3 проверены пофайлово:** `notify_approve_requested`, `notify_error`, +`stage_engine.py` (все alert'ы с номером), `agents/launcher.py`, `security_gate.py`, +`reconciler.py` — номер кликабелен. `merge_gate.py`/`job_reaper.py`/`main.py` оставлены без +ссылки **осознанно и корректно**: их тексты ссылаются на repo/job/run_id, а НЕ на +`work_item_id` (проверено: merge_gate:432 — lease/repo, job_reaper:396 — job/agent/repo, +main:47 — orphaned run_ids). + +**Инварианты/нерегресс:** схема БД, `STAGE_TRANSITIONS`, QG, транспорт — не тронуты +(AC-15 ✓). `get_db()` возвращает новое соединение на вызов, поэтому `conn.close()` в +`link_for` корректен. `pytest tests/ -q` → **907 passed** (AC-16, AC-17 ✓). + +**Документация (блокеры v1 закрыты):** `CHANGELOG.md`, `CLAUDE.md`, `.env.example` +обновлены в коммите `7a88f39`; ADR-001 присутствует и полон; `README.md`/`internals.md` +синхронизированы (AC-18 ✓). ## Findings ### P0 — Blocker -- [ ] **`CHANGELOG.md` не обновлён.** Нет записи ORCH-067 в секции `## [Unreleased]` - (`grep -c ORCH-067 CHANGELOG.md` → 0). Требуется ТЗ §5 и AC-18. Добавить запись о - смене дефолта `bump`, статус-строке карточки и кликабельном номере. -- [ ] **`CLAUDE.md` не обновлён.** ТЗ §5 (стр. 183) требует «раздел про нотификации/tracker - (дефолт bump; статус-строка карточки; кликабельный номер в карточке и уведомлениях)». - Файл не менялся, ORCH-067 в нём не упоминается. AC-18 → FAIL. +- (нет) ### P1 — Must fix -- [ ] **`.env.example` рассинхронизирован.** ТЗ §4 (и §1.3): «Обновить `.env.example`, если в - нём фигурируют `ORCH_TRACKER_MODE`». Строка осталась `ORCH_TRACKER_MODE=edit`, тогда как - дефолт сменён на `bump` — канон env вводит в заблуждение. Также отсутствуют новые флаги - из `config.py` (`ORCH_TRACKER_LIVE_STATUS`, `ORCH_TRACKER_LIVE_STATUS_TTL_S`, - `ORCH_TRACKER_LIVE_STATUS_TIMEOUT_S`) — желательно задокументировать там же. +- (нет) ### P2 — Should fix - (нет) +### P3 — Nice to have (не блокирует) +- [ ] Часть alert-сообщений в `stage_engine.py` (`_handle_self_deploy_phase_b`, + `_handle_merge_verify`) встраивает «сырой» `{msg}`/`{e}`/`{reason}` рядом с новой + ``-ссылкой; под `parse_mode=HTML` редкий `<` в этих подстановках теоретически мог + бы помешать рендеру. Это **пре-существующее поведение** (parse_mode=HTML стоял и + раньше), не регресс данной задачи; `notify_error` свой `error` экранирует. Можно при + случае обернуть прочие подстановки в `html.escape`. + ## Документация -- `docs/architecture/README.md` — обновлён (новый компонент Notifications / Live-tracker). ✓ -- `docs/architecture/internals.md` — обновлён (§7: режимы bump/edit, строка Plane-статуса, - кликабельный номер). Качественно и точно. ✓ -- ADR per-work-item `06-adr/ADR-001-tracker-plane-status-and-link.md` — присутствует, полный. ✓ -- `CHANGELOG.md` — **НЕ обновлён** (P0). -- `CLAUDE.md` — **НЕ обновлён** (P0). -- `.env.example` — **НЕ синхронизирован** с новым дефолтом и флагами (P1). +- `docs/architecture/README.md` — обновлён (компонент Notifications / live-tracker). ✓ +- `docs/architecture/internals.md` — обновлён (§7: bump/edit, Plane-статус, кликабельный номер). ✓ +- `06-adr/ADR-001-tracker-plane-status-and-link.md` — присутствует, полный, закрывает все `[ARCH]`. ✓ +- `CHANGELOG.md` — обновлён (запись ORCH-067). ✓ +- `CLAUDE.md` — обновлён (раздел «Нотификации / Telegram live-tracker»). ✓ +- `.env.example` — синхронизирован (`ORCH_TRACKER_MODE=bump` + новые флаги live-overlay). ✓ -После закрытия P0/P1 (CHANGELOG + CLAUDE.md + .env.example) задача готова к повторному ревью. -Код и тесты правок не требуют. +Документация = golden source: код и доку обновлены в одном PR. Блокеры предыдущего ревью +(v1) закрыты. Замечаний уровня P0/P1/P2 нет → **APPROVED**.