reviewer(ET): auto-commit from reviewer run_id=444
This commit is contained in:
91
docs/work-items/ORCH-089/12-review.md
Normal file
91
docs/work-items/ORCH-089/12-review.md
Normal file
@@ -0,0 +1,91 @@
|
||||
---
|
||||
type: review
|
||||
work_item_id: ORCH-089
|
||||
verdict: APPROVED
|
||||
version: 1
|
||||
---
|
||||
|
||||
# Review ORCH-089
|
||||
|
||||
## Summary
|
||||
|
||||
Авто-режим по лейблам Plane (`autoApprove` + `autoDeploy`) реализован строго по ТЗ
|
||||
и ADR: аддитивно, по образцу условных под-гейтов (ORCH-035/043/058/088). Снимаются
|
||||
**только два человеческих решения** (гейт BRD `Approved`, гейт прод-деплоя
|
||||
`Confirm Deploy`); ни одна техническая проверка не тронута. Соответствие всем осям
|
||||
(ТЗ, ADR, качество кода, тесты) — полное; документация-golden-source обновлена в том
|
||||
же PR. Блокирующих findings нет. **Вердикт: APPROVED.**
|
||||
|
||||
## Проверка по осям
|
||||
|
||||
### 1. Соответствие ТЗ (`02-trz.md`)
|
||||
- ✅ Leaf `src/labels.py` (never-raise): `auto_approve_applies`/`auto_deploy_applies`
|
||||
(локальный scope-чек ПЕРВЫМ), `has_label` (единственный сетевой вызов, только при
|
||||
`applies==True` → нулевой оверхед при выключенном флаге, §7/AC-8), `snapshot`.
|
||||
- ✅ `src/plane_sync.py`: `fetch_issue_labels` (`None` при ошибке ≠ `[]`),
|
||||
`get_project_labels` (`{normalized→uuid}`, TTL-кэш `auto_label_states_ttl_s` по
|
||||
образцу `get_project_states`, сентинел `__AMBIGUOUS__` при коллизии имён),
|
||||
`set_issue_approved` (1:1 зеркало `set_issue_in_review`).
|
||||
- ✅ Врезка autoApprove — `_handle_analysis_approved_flow` (ветка `files_ok`) ПОСЛЕ
|
||||
`In Review`+коммента; advance идёт через тот же `advance_stage(..., finished_agent=None)`,
|
||||
что человеческий Approved (без дублирования переходной логики, §3.1).
|
||||
- ✅ Врезка autoDeploy — `_handle_self_deploy_phase_a` после advance на `deploy`+
|
||||
`clear_state`, ДО «ask-human»; Phase B запускается тем же `_handle_self_deploy_phase_b`
|
||||
(§3.2).
|
||||
- ✅ Флаги (`config.py`): `auto_label_enabled`, `auto_approve_label`, `auto_deploy_label`,
|
||||
`auto_label_repos` (пусто → self-hosting only), `auto_label_states_ttl_s` (§7).
|
||||
- ✅ Блок наблюдаемости `auto_labels` в `GET /queue` (§8).
|
||||
- ✅ БД-схема и QG-реестр не трогаются (§5/§6) — подтверждено: `stages.py`,
|
||||
`qg/checks.py`, `db.py` отсутствуют в diff feat-коммита.
|
||||
|
||||
### 2. Соответствие ADR (`06-adr/ADR-001`, global `adr-0018`)
|
||||
- ✅ Реализация 1:1 с решениями ADR: D1 (поверхность leaf + порядок резолва `has_label`),
|
||||
D5 (scope: пусто → self-hosting), fail-safe «never auto on doubt», ambiguity-сентинел.
|
||||
- ✅ Глобальный сквозной ADR `adr-0018-auto-label-gates.md` заведён.
|
||||
- ✅ Подтверждена корректность пути advance: `advance_stage` с `agent=None` идёт в
|
||||
ветку `approved-via-status` (qg_passed, без повторного `check_analysis_approved`) →
|
||||
`analysis → architecture` + `mark_brd_review_ended`. Re-entrancy безопасна
|
||||
(вложенный вызов с `finished_agent=None` не входит в analyst-ветку).
|
||||
|
||||
### 3. Качество кода
|
||||
- ✅ never-raise соблюдён во всех публичных функциях (`labels.py`, новые `plane_sync`-хелперы).
|
||||
- ✅ Нет дублирования переходной логики — переиспользованы `advance_stage` и
|
||||
`_handle_self_deploy_phase_b` (включая существующую идемпотентность `INITIATED`).
|
||||
- ✅ Прозрачность (AC-7) во всех трёх каналах: лог + Telegram (`send_telegram`) +
|
||||
Plane-коммент (`plane_add_comment`), плюс live-карточка через штатный advance.
|
||||
- ✅ Docstrings содержательные; кликабельный номер задачи (`link_for`) в уведомлениях.
|
||||
|
||||
### 4. Тесты
|
||||
- ✅ 43 целевых теста (TC-01…TC-26, 7 модулей) — все зелёные.
|
||||
- ✅ Регрессия: 377 релевантных тестов (stage/plane/analysis/deploy/self_deploy/webhook)
|
||||
— все зелёные. AC-10 (инварианты) подтверждён.
|
||||
|
||||
## Документация
|
||||
Обновлена полностью в том же PR (AC-11):
|
||||
- `CLAUDE.md` — раздел «Авто-режим по лейблам» (флаги, инвариант «снимает только
|
||||
человеческое решение»);
|
||||
- `docs/architecture/README.md` — описание врезок autoApprove/autoDeploy + флаги;
|
||||
- `CHANGELOG.md` — запись в `## [Unreleased]`;
|
||||
- `06-adr/ADR-001-auto-label-gates.md` + global `docs/architecture/adr/adr-0018-auto-label-gates.md`;
|
||||
- `07-infra-requirements.md` — предусловие создания лейблов `autoApprove`/`autoDeploy`
|
||||
в Plane-проекте ORCH.
|
||||
|
||||
Статус: документация синхронна с кодом. Требование CLAUDE.md §2/§6 выполнено.
|
||||
|
||||
## Findings
|
||||
|
||||
### P0 — Blocker
|
||||
- Нет.
|
||||
|
||||
### P1 — Must fix
|
||||
- Нет.
|
||||
|
||||
### P2 — Should fix
|
||||
- Нет.
|
||||
|
||||
### P3 — Nice-to-have
|
||||
- `set_issue_approved` обращается к `get_project_states(pid)["approved"]` прямым
|
||||
индексом (потенциальный `KeyError`, если ключ отсутствует). На практике защищено:
|
||||
ключ `approved` гарантирован в `_DEFAULT_STATES`, паттерн 1:1 повторяет
|
||||
существующий `set_issue_in_review`, а вызов обёрнут внешним `try/except` в
|
||||
`advance_stage` (деградирует к ручному гейту). Косметика, не блокер.
|
||||
Reference in New Issue
Block a user