reviewer(ET): auto-commit from reviewer run_id=199
This commit is contained in:
@@ -1,28 +1,37 @@
|
|||||||
---
|
---
|
||||||
type: review
|
type: review
|
||||||
work_item_id: ORCH-036
|
work_item_id: ORCH-036
|
||||||
verdict: REQUEST_CHANGES
|
verdict: APPROVED
|
||||||
version: 1
|
version: 2
|
||||||
---
|
---
|
||||||
|
|
||||||
# Review ORCH-036 — Исполняемый самодеплой стадии `deploy` (Вариант B)
|
# Review ORCH-036 — Исполняемый самодеплой стадии `deploy` (Вариант B)
|
||||||
|
|
||||||
## Summary
|
## Summary
|
||||||
|
|
||||||
Реализация трёхфазного исполняемого самодеплоя (`src/self_deploy.py` + перехваты
|
Re-review после фикса двух P1 из версии 1. Оба блокера устранены:
|
||||||
Фаз 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) обновлена
|
|
||||||
содержательно.
|
|
||||||
|
|
||||||
Однако обнаружены **два P1**, блокирующие приёмку: (1) sentinel-маркеры не
|
1. **Stale deploy-state маркеры** — добавлен `self_deploy.clear_state(repo, work_item_id)`
|
||||||
очищаются на откате → повторный прод-деплой после фикса невозможен (конвейер
|
(never-raise, idempotent, рекурсивное удаление `<repos_dir>/.deploy-state-<repo>/<wi>/`)
|
||||||
зависает на `deploy`); (2) не обновлён канонический `.env.example` вопреки прямому
|
в ветке БАГ-8-отката `check_deploy_status` FAILED (`_handle_qg_failure_rollbacks`,
|
||||||
требованию ТЗ §2.6 и правилу CLAUDE.md №8.
|
`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
|
## Findings
|
||||||
|
|
||||||
@@ -30,57 +39,26 @@ DEPLOY_HOOK.md, CHANGELOG, ADR-001 + глобальный adr-0007, README) об
|
|||||||
- (нет)
|
- (нет)
|
||||||
|
|
||||||
### P1 — Must fix
|
### P1 — Must fix
|
||||||
|
- (нет — оба P1 из версии 1 устранены и покрыты тестами)
|
||||||
- [ ] **Stale deploy-state маркеры ломают re-deploy после отката (`src/stage_engine.py`,
|
|
||||||
`src/self_deploy.py`).** Маркеры `initiated` / `result` / `approve-requested`
|
|
||||||
лежат под `<repos_dir>/.deploy-state-<repo>/<work_item_id>/` и ключуются по
|
|
||||||
**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 (значения — плейсхолдеры, секреты не коммитить).
|
|
||||||
|
|
||||||
### P2 — Should fix
|
### P2 — Should fix
|
||||||
- [ ] (опц.) INFRA.md обновлён в части env-карты, но пошаговой процедуры
|
- (нет блокирующих; прежний P2 про сквозную процедуру оператора частично закрыт:
|
||||||
«approve → прод-деплой через хук» в INFRA нет (есть в deployer.md и
|
env-карта новых настроек добавлена в INFRA.md, пошаговый approve→deploy описан в
|
||||||
DEPLOY_HOOK.md). Желательно дать короткую сквозную процедуру оператору в одном
|
deployer.md и DEPLOY_HOOK.md)
|
||||||
месте. Не блокирует.
|
|
||||||
|
|
||||||
## Документация
|
## Документация
|
||||||
|
|
||||||
Обновлена содержательно и в том же PR: `.openclaw/agents/deployer.md` (стадия
|
Обновлена содержательно и в том же PR:
|
||||||
`deploy` = вызов хука, явный запрет self-restart 8500), `docs/operations/INFRA.md`
|
- `.openclaw/agents/deployer.md` — стадия `deploy` переписана: self-hosting путь
|
||||||
(env-карта новых настроек), `docs/operations/DEPLOY_HOOK.md` (`SOURCE_IMAGE`
|
(Фазы A/B/C, явный запрет рестарта 8500 изнутри агента) vs прежний синхронный
|
||||||
build-once + прод-пример), `docs/architecture/README.md` (раздел про исполняемый
|
ssh-путь для не-self репо;
|
||||||
самодеплой), `CHANGELOG.md` (детальная запись), ADR-001 в `06-adr/` + глобальный
|
- `docs/operations/INFRA.md` — env-карта всех новых `ORCH_SELF_DEPLOY_*` / `ORCH_DEPLOY_*`;
|
||||||
`docs/architecture/adr/adr-0007-executable-self-deploy.md`.
|
- `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 и
|
Документация = golden source: изменения `src/` сопровождены синхронным обновлением
|
||||||
прямое требование ТЗ §2.6 — НЕ обновлён новыми `ORCH_DEPLOY_*` / `ORCH_SELF_DEPLOY_*`.
|
доки в том же PR. Ось документации — PASS.
|
||||||
Это и есть основание держать вердикт `REQUEST_CHANGES` по оси документации, в
|
|
||||||
дополнение к функциональному P1 по очистке маркеров.
|
|
||||||
|
|||||||
Reference in New Issue
Block a user