diff --git a/docs/work-items/ORCH-036/12-review.md b/docs/work-items/ORCH-036/12-review.md index 00c3252..f44520e 100644 --- a/docs/work-items/ORCH-036/12-review.md +++ b/docs/work-items/ORCH-036/12-review.md @@ -1,28 +1,37 @@ --- type: review work_item_id: ORCH-036 -verdict: REQUEST_CHANGES -version: 1 +verdict: APPROVED +version: 2 --- # Review ORCH-036 — Исполняемый самодеплой стадии `deploy` (Вариант B) ## Summary -Реализация трёхфазного исполняемого самодеплоя (`src/self_deploy.py` + перехваты -Фаз A/B/C в `src/stage_engine.py`, reserved-agent `deploy-finalizer` в -`src/agents/launcher.py`, build-once `SOURCE_IMAGE` в хуке) — архитектурно -соответствует ADR-001 и закрывает большинство критериев приёмки. Код аккуратный, -defensive (never-raise), 566/566 тестов зелёные, контракты `STAGE_TRANSITIONS` / -`_parse_deploy_status` / БАГ-8 / merge-gate не тронуты, условность по репо -(`self_deploy_applies`) корректна, документация (deployer.md, INFRA.md, -DEPLOY_HOOK.md, CHANGELOG, ADR-001 + глобальный adr-0007, README) обновлена -содержательно. +Re-review после фикса двух P1 из версии 1. Оба блокера устранены: -Однако обнаружены **два P1**, блокирующие приёмку: (1) sentinel-маркеры не -очищаются на откате → повторный прод-деплой после фикса невозможен (конвейер -зависает на `deploy`); (2) не обновлён канонический `.env.example` вопреки прямому -требованию ТЗ §2.6 и правилу CLAUDE.md №8. +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**. ## Findings @@ -30,57 +39,26 @@ DEPLOY_HOOK.md, CHANGELOG, ADR-001 + глобальный adr-0007, README) об - (нет) ### P1 — Must fix - -- [ ] **Stale deploy-state маркеры ломают re-deploy после отката (`src/stage_engine.py`, - `src/self_deploy.py`).** Маркеры `initiated` / `result` / `approve-requested` - лежат под `/.deploy-state-//` и ключуются по - **work_item_id**, который стабилен на всю жизнь задачи. БАГ-8 откат - (`_handle_qg_failure_rollbacks`, ветка `deployer`+`check_deploy_status`, - строки 620–647) НЕ удаляет эти маркеры, и нигде в `src/` нет их очистки - (проверено grep'ом). Сценарий: deploy FAILED → откат на development → developer - чинит → задача снова доходит до `deploy` → человек ставит Approved → Фаза B - (`_handle_self_deploy_phase_b`) видит существующий `initiated` (от первой - попытки) → возвращает `self-deploy-already-initiated` **no-op**, detached-хук НЕ - запускается, finalizer НЕ ставится. Задача навсегда зависает на `deploy`, - ожидая approve, который ничего не делает. Дополнительно: устаревший `result` - (FAILED exit от первой попытки) тоже остаётся и был бы прочитан новым - finalizer'ом. Это ломает retry-контракт самой стадии для self-hosting-репо — - ровно тот класс, под который сделана фича (AC-4 работает только на ПЕРВЫЙ - провал; AC-10 «БАГ-8 rollback+retry не сломан» нарушается на втором проходе). - Нужно очищать маркеры (`initiated`, `result`, `approve-requested`) на откате - `deploy → development` (и/или в начале Фазы B/Фазы A для нового прохода), плюс - тест на сценарий FAILED → rollback → повторный deploy. - -- [ ] **`.env.example` не обновлён (`/.env.example`).** ТЗ §2.6 прямо требует: - «Прописать дескрипторы в `.env.example`», CLAUDE.md правило №8 объявляет - `.env.example` каноном env-конфигурации, а предыдущая задача ORCH-043 - (merge-gate) задала прецедент — добавила свои `ORCH_MERGE_*` именно туда с - описаниями. ORCH-036 ввёл ~14 новых настроек в `src/config.py` - (`self_deploy_enabled`, `self_deploy_repos`, `deploy_require_manual_approve`, - `deploy_ssh_user/host`, `deploy_hook_script`, `deploy_host_repo_path`, - `deploy_prod_*`, `deploy_finalize_delay_s/max_attempts`) и задокументировал их в - таблице INFRA.md, но в `.env.example` они отсутствуют полностью. Оператор, - поднимающий сервис по канону `.env.example`, не узнает про обязательный - `ORCH_DEPLOY_SSH_HOST` (дефолт пустой → detached-деплой не запустится). Добавить - блок `ORCH_SELF_DEPLOY_*` / `ORCH_DEPLOY_*` с дескрипторами по образцу - merge-gate (значения — плейсхолдеры, секреты не коммитить). +- (нет — оба P1 из версии 1 устранены и покрыты тестами) ### P2 — Should fix -- [ ] (опц.) INFRA.md обновлён в части env-карты, но пошаговой процедуры - «approve → прод-деплой через хук» в INFRA нет (есть в deployer.md и - DEPLOY_HOOK.md). Желательно дать короткую сквозную процедуру оператору в одном - месте. Не блокирует. +- (нет блокирующих; прежний P2 про сквозную процедуру оператора частично закрыт: + env-карта новых настроек добавлена в INFRA.md, пошаговый approve→deploy описан в + deployer.md и DEPLOY_HOOK.md) ## Документация -Обновлена содержательно и в том же PR: `.openclaw/agents/deployer.md` (стадия -`deploy` = вызов хука, явный запрет self-restart 8500), `docs/operations/INFRA.md` -(env-карта новых настроек), `docs/operations/DEPLOY_HOOK.md` (`SOURCE_IMAGE` -build-once + прод-пример), `docs/architecture/README.md` (раздел про исполняемый -самодеплой), `CHANGELOG.md` (детальная запись), ADR-001 в `06-adr/` + глобальный -`docs/architecture/adr/adr-0007-executable-self-deploy.md`. +Обновлена содержательно и в том же 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_*`; +- `docs/operations/DEPLOY_HOOK.md` — `SOURCE_IMAGE` build-once + прод-пример; +- `docs/architecture/README.md` — раздел «Исполняемый самодеплой стадии `deploy`»; +- `CHANGELOG.md` — запись Added (фича) + запись Fixed (review-fix: clear_state + .env.example); +- 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). -**Пробел (см. P1):** `.env.example` — канонический шаблон env по CLAUDE.md №8 и -прямое требование ТЗ §2.6 — НЕ обновлён новыми `ORCH_DEPLOY_*` / `ORCH_SELF_DEPLOY_*`. -Это и есть основание держать вердикт `REQUEST_CHANGES` по оси документации, в -дополнение к функциональному P1 по очистке маркеров. +Документация = golden source: изменения `src/` сопровождены синхронным обновлением +доки в том же PR. Ось документации — PASS.