diff --git a/docs/work-items/ORCH-036/12-review.md b/docs/work-items/ORCH-036/12-review.md index f44520e..1fafc9e 100644 --- a/docs/work-items/ORCH-036/12-review.md +++ b/docs/work-items/ORCH-036/12-review.md @@ -2,36 +2,52 @@ type: review work_item_id: ORCH-036 verdict: APPROVED -version: 2 +version: 3 --- # Review ORCH-036 — Исполняемый самодеплой стадии `deploy` (Вариант B) ## Summary -Re-review после фикса двух P1 из версии 1. Оба блокера устранены: +Полное ревью реализации после слияния ветки с `origin/main` (merge-commit `36c1898`; +параллельно в `main` приехал ORCH-053 reconciler). Реализация трёхфазного исполняемого +самодеплоя соответствует ТЗ (`02-trz.md`), критериям приёмки (`03-acceptance-criteria.md` +AC-1…AC-13) и ADR-001. Блокеров нет. -1. **Stale deploy-state маркеры** — добавлен `self_deploy.clear_state(repo, work_item_id)` - (never-raise, idempotent, рекурсивное удаление `/.deploy-state-//`) - в ветке БАГ-8-отката `check_deploy_status` FAILED (`_handle_qg_failure_rollbacks`, - `src/stage_engine.py`) и дополнительно в начале Фазы A (`_handle_self_deploy_phase_a`) - как belt-and-suspenders. Добавлен регрессионный тест - `tests/test_deploy_rollback.py::test_tc11_re_deploy_after_rollback_not_wedged`, - доказывающий, что после FAILED → откат → фикс → повторный заход на `deploy` Фаза B - РЕАЛЬНО инициирует деплой (нет no-op по устаревшему `initiated`), плюс - `tests/test_deploy_hook_mapping.py::test_clear_state_removes_all_markers_and_is_idempotent`. -2. **`.env.example`** — добавлен полный блок дескрипторов `ORCH_SELF_DEPLOY_*` / - `ORCH_DEPLOY_*` (14 настроек, плейсхолдеры, секреты не коммитятся) по образцу - merge-gate ORCH-043, с подробными комментариями. +Проверено по четырём осям: -Реализация трёхфазного исполняемого самодеплоя соответствует ADR-001 и закрывает -критерии приёмки AC-1…AC-13. Контракты `STAGE_TRANSITIONS` / `QG_CHECKS` / -`_parse_deploy_status` / БАГ-8 / terminal-sync / merge-gate (ORCH-43) НЕ тронуты; -условность по репо (`self_deploy_applies`) корректна; перехваты упорядочены верно -(Phase B после terminal-check, Phase A после merge-gate); `deploy-finalizer` — -детерминированный no-LLM reserved-agent, перехвачен в launcher до `_spawn`. Все -импорты (`set_issue_in_review`, `plane_add_comment`, `set_issue_blocked`, -`send_telegram`) присутствуют. `pytest tests/` — **568 passed**. +**1. Соответствие ТЗ.** Все требования §2.1–§2.7 закрыты: build-once `SOURCE_IMAGE`-retag +в хуке (§2.1, обратно совместимо, exit-code-контракт цел); approve-гейт по флагу +`deploy_require_manual_approve=true` (§2.2); detached ssh+setsid-триггер из стадии `deploy` +(§2.3, агент себя не рестартит — BR-2); маппинг exit→`deploy_status` чистой функцией +`map_exit_code_to_status` (§2.4); Plane+Telegram на approve-request/initiate/success/rollback +(§2.5, BR-5); конфиг + `.env.example`-дескрипторы (§2.6); документация (§2.7). + +**2. Соответствие ADR.** Дизайн A/B/C реализован дословно: Фаза A перехватывает на ребре +`deploy-staging→deploy` ПОСЛЕ `check_staging_status` и merge-gate (порядок в `advance_stage` +верный — проверено строки 203-288); Фаза B — дискриминатор `finished_agent is None` + +идемпотентность по маркеру `initiated`; Фаза C — reserved-agent `deploy-finalizer`, +перехвачен в `launcher.launch_job` ДО `_spawn` (R-6), drive существующих контрактов через +`advance_stage(..., finished_agent="deployer")`. Глобальные ADR не нарушены. +**Контракты НЕ тронуты (AC-10):** `STAGE_TRANSITIONS`, `QG_CHECKS`, `check_deploy_status`/ +`_parse_deploy_status` (frontmatter-only), terminal-sync `deploy→done`, merge-gate (ORCH-43), +откат БАГ-8. + +**3. Качество кода.** `src/self_deploy.py` — чистый leaf-модуль, контракт never-raise на +каждом публичном хелпере; sentinel-state restart-safe без миграции БД; `clear_state` в +БАГ-8-откате + belt-and-suspenders в Фазе A снимает класс «stale `initiated` → wedge при +повторном заходе на deploy» (AC-4). Условность `self_deploy_applies` зеркалит ORCH-35/43 +(kill-switch + CSV + self-hosting fallback). Интеграционные точки сверены: +`enqueue_job(available_at_delay_s=, task_id=)`, `mark_job`, `get_worktree_path`, +`repos_dir`/`host_repos_dir`, импорты `set_issue_in_review`/`set_issue_blocked`/ +`plane_add_comment`/`send_telegram` — все присутствуют. Defer-бюджет finalizer считается +из `jobs` по `task_content LIKE '%deploy-finalize defer%'` (restart-safe, начальный +poll-job не засчитывается — корректно). Fail-closed маппинг (любой не-0 / мусор → FAILED). + +**4. Качество тестов.** `pytest tests/` — **596 passed**. Покрытие содержательное: +exit-mapping (TC-01/02/03), approve-гейт, routing self/non-self, rollback + +re-deploy-after-rollback, notifications, build-once, terminal-sync, staging-precondition, +hook-rollback-sim. ## Findings @@ -39,26 +55,40 @@ Re-review после фикса двух P1 из версии 1. Оба блок - (нет) ### P1 — Must fix -- (нет — оба P1 из версии 1 устранены и покрыты тестами) +- (нет) ### P2 — Should fix -- (нет блокирующих; прежний P2 про сквозную процедуру оператора частично закрыт: - env-карта новых настроек добавлена в INFRA.md, пошаговый approve→deploy описан в - deployer.md и DEPLOY_HOOK.md) +- [ ] **Коллизия номера глобального ADR.** В `docs/architecture/adr/` сосуществуют + `adr-0007-executable-self-deploy.md` (ORCH-036) и `adr-0007-reconciler.md` (ORCH-053) — + два разных решения под одним номером 0007 (артефакт параллельного слияния ORCH-053 в + `main`). Канон CLAUDE.md требует уникальной нумерации `adr-NNNN-slug.md`. Рекомендация: + перенумеровать одно из решений (напр. self-deploy → `adr-0008`) и поправить ссылки в + `docs/architecture/README.md`. Не блокер: на функционал/контракты не влияет. +- [ ] **Дублирующийся футер `README.md`.** `docs/architecture/README.md` строки 169-170 — + две почти идентичные строки «*Актуально на 2026-06-06…*» (merge-артефакт ORCH-036↔ORCH-053). + Оставить одну консолидированную строку. + +### P3 — Nice to have +- (нет) ## Документация -Обновлена содержательно и в том же PR: -- `.openclaw/agents/deployer.md` — стадия `deploy` переписана: self-hosting путь - (Фазы A/B/C, явный запрет рестарта 8500 изнутри агента) vs прежний синхронный - ssh-путь для не-self репо; -- `docs/operations/INFRA.md` — env-карта всех новых `ORCH_SELF_DEPLOY_*` / `ORCH_DEPLOY_*`; +Обновлена содержательно и в том же PR (ось документации — **PASS**): +- `.openclaw/agents/deployer.md` — стадия `deploy` переписана: self-hosting путь (Фазы A/B/C, + явный запрет рестарта 8500 изнутри агента) vs прежний синхронный ssh-путь для не-self репо; +- `docs/architecture/README.md` — новый раздел «Исполняемый самодеплой стадии `deploy` (ORCH-36)»; +- `docs/operations/INFRA.md` — env-карта новых `ORCH_SELF_DEPLOY_*` / `ORCH_DEPLOY_*`; - `docs/operations/DEPLOY_HOOK.md` — `SOURCE_IMAGE` build-once + прод-пример; -- `docs/architecture/README.md` — раздел «Исполняемый самодеплой стадии `deploy`»; -- `CHANGELOG.md` — запись Added (фича) + запись Fixed (review-fix: clear_state + .env.example); +- `CHANGELOG.md` — запись Added (ORCH-036); - ADR `docs/work-items/ORCH-036/06-adr/ADR-001-executable-self-deploy.md` + глобальный - `docs/architecture/adr/adr-0007-executable-self-deploy.md`; -- **`.env.example`** — канонический шаблон (CLAUDE.md №8, ТЗ §2.6) дополнен (был пробел в v1). + `docs/architecture/adr/adr-0007-executable-self-deploy.md` (см. P2 о номере); +- `.env.example` — блок дескрипторов настроек (секреты не коммитятся, канон CLAUDE.md №8). -Документация = golden source: изменения `src/` сопровождены синхронным обновлением -доки в том же PR. Ось документации — PASS. +`src/` сопровождён синхронным обновлением документации в том же PR. Найденные P2 — это +консистентность нумерации/футера, возникшая из-за параллельного слияния, а не отсутствие +документации. + +## Вердикт + +Нет P0/P1 → **APPROVED**. P2 (коллизия номера ADR, дубль футера README) рекомендуется +устранить отдельным docs-PR; конвейер не блокируют.