146 lines
14 KiB
Markdown
146 lines
14 KiB
Markdown
---
|
||
verdict: APPROVED
|
||
work_item: ORCH-009
|
||
stage: review
|
||
author_agent: reviewer
|
||
status: approved
|
||
created_at: 2026-06-10
|
||
model_used: claude-fable-5
|
||
type: review
|
||
work_item_id: ORCH-009
|
||
version: 1
|
||
---
|
||
|
||
# Review ORCH-009 — Turnkey-онбординг проектов (kit + CLI + runbook)
|
||
|
||
Ветка: `feature/ORCH-009-turnkey-plane` · Diff vs `origin/main`: 41 файл, +5120/−10.
|
||
Состав: kit `onboarding/repo-skeleton/` (28 файлов), CLI `scripts/onboard_project.py` (1090 строк),
|
||
runbook `docs/operations/ONBOARDING.md`, 3 тест-модуля (83 теста), golden-source доки, ADR×2.
|
||
|
||
## Summary
|
||
|
||
PR реализует ТЗ полностью и точно по ADR-001 (D1–D11), с нулевым касанием рантайма. Вердикт
|
||
**APPROVED**: P0/P1 findings нет; найдены 3×P2 (харднинг краевых путей CLI) и 2×P3 (косметика) —
|
||
не блокируют, рекомендованы к follow-up. Документация обновлена в том же PR по всем требуемым
|
||
точкам.
|
||
|
||
**Проверено по 4 осям:**
|
||
|
||
### Ось 1 — Соответствие ТЗ (`02-trz.md`, `03-acceptance-criteria.md`) — ✅
|
||
|
||
| Требование | Статус | Чем подтверждено |
|
||
|---|---|---|
|
||
| FR-1 состав kit | ✅ | AC-1/TC-01: все 19 элементов на месте; `_templates`/`_standards` в kit НЕ хранятся (анти-форк тест) |
|
||
| FR-2 канон 52d/92 промптов | ✅ | AC-2/TC-03…06: 5 XML-секций в нормативном порядке, «❌→✅», `<escalation>` у dev/reviewer/tester, `<thinking>` — паритет с эталоном (architect/reviewer/tester/deployer в обоих деревьях), 52c-схема, verdict-ключи байт-в-байт, даты/модели — плейсхолдеры |
|
||
| FR-2 reviewer-gate доки | ✅ | AC-3/TC-07: «документация НЕ обновлена → `REQUEST_CHANGES`» в kit reviewer.md |
|
||
| FR-3 INFRA.md шаблон | ✅ | AC-10/TC-19: топология/контейнеры/env-карта/границы доступа/риски общего хоста/деплой |
|
||
| FR-4 CLI plan/apply/verify | ✅ | AC-7/8/9, TC-13…18: 22 статуса из `_PLANE_NAME_TO_KEY`, группы по D5, лейблы из конфига, dry-run без единой мутации (мутации materialize/push замоканы на AssertionError), идемпотентный ensure, delete-операций нет, `docker`/`systemctl`/`compose`/запись `.env` отсутствуют в исходнике (TC-18-тест по AST/grep) |
|
||
| FR-5 verify | ✅ | round-trip реестра фактическим парсером, резолв всех 22 имён (включая fail-closed `Confirm Deploy`/`STOP` — отдельный негативный тест), лейблы, webhook active, полнота kit, скан `{{…}}` |
|
||
| FR-6 runbook | ✅ | AC-11/TC-20: все слои в порядке, 🖐-маркировка ручных шагов + команды проверки, self-hosting-предупреждение о групповом окне рестарта, workspace-webhook — «существует, только проверка» |
|
||
| §4/§5 нет API/БД изменений | ✅ | diff: `src/**` пуст |
|
||
| §9 инварианты | ✅ | см. ось 2 |
|
||
| AC-12 регресс | ✅ | 1794 passed локально; 2 падения (`test_resolve_agent_model`/`_effort`) — **средовые, pre-existing**: вызваны `ORCH_AGENT_FALLBACK_MODEL`/`ORCH_AGENT_EFFORT_*` в env агент-раннера, с очищенной средой 49/49 зелёные; файлы этих тестов PR не трогает — не регресс этого PR (авторитетен CI с чистой средой) |
|
||
| AC-13 операторский smoke | ⏳ | По построению выполняется на приёмке (tester/оператор): runbook §5.2 + «Журнал smoke-прогонов» (плейсхолдер первого прогона), D8 требует ссылку из `13-test-report.md`. **Handoff-заметка стадии testing** — см. «Для следующей стадии» |
|
||
|
||
### Ось 2 — Соответствие ADR (`06-adr/ADR-001`, сквозной `adr-0035`) — ✅
|
||
|
||
- **D1–D11 реализованы без отступлений**: раскладка top-level `onboarding/` (D1); `{{NAME}}` +
|
||
`str.replace` + обязательный пост-скан + биекция словаря (D2, тест); live-copy verbatim
|
||
`_templates`(≥16)/`_standards`(≥3) (D3, тест байт-сравнения); закрытый список импортов `src` —
|
||
AST-тест `test_tc21_cli_src_imports_stay_in_closed_list` (D4); таблица групп `STATE_GROUPS` 1:1
|
||
с таблицей D5, код-критичные констрейнты загвождены тестом (`STOP`→`cancelled`; терминальные
|
||
группы == {Done, Cancelled, STOP}; set-равенство с `_PLANE_NAME_TO_KEY` ловит будущий дрейф);
|
||
`auto_init=false` + переиспользование глобального секрета + push только в пустой репо (D6 —
|
||
сверил с приёмником `src/webhooks/gitea.py:38-41`: действительно ОДИН глобальный секрет);
|
||
merged-full-array + round-trip + «.env не правим» (D7); smoke на staging 8501 (D8); 5 ru +
|
||
deployer en c «Do NOT translate»-гардом (D9, тест на кириллицу); runbook фиксирует «branch
|
||
protection НЕ включать» (D10); plan/apply/verify, чистый `build_plan`, инжектируемые клиенты,
|
||
отчёт `created/skipped(exists)/manual-step/planned/error`, exit-коды 0/2/1 (D11).
|
||
- **Инварианты NFR-1/INV-4**: `git diff origin/main...HEAD -- src/ .openclaw/ docs/_templates/
|
||
docs/_standards/ docs/operations/INFRA.md` — пусто; снапшот-тесты `STAGE_TRANSITIONS`/`QG_CHECKS`
|
||
зелёные; push — только initial в свежесозданный пустой репо (вне конвейера до регистрации),
|
||
PR-merge API не затрагивается.
|
||
- **Трассировка (TRACEABILITY.md)**: правка `docs/operations/SETUP_WEBHOOKS.md` обобщает
|
||
enduro-хардкод, **усиливая** (не ломая) инвариант одного глобального HMAC-секрета — сверено с
|
||
кодом приёмника; правка `docs/architecture/adr/README.md` — реестр сквозных ADR (общий индекс),
|
||
бэкфилл строк 0032–0034 сверен: все три файла существуют в main, номера/задачи корректны,
|
||
«текущий максимум 0035» верен. Чужие маркированные инварианты не задеты.
|
||
|
||
### Ось 3 — Качество кода — ✅ (с P2-findings ниже)
|
||
|
||
Docstrings на всех публичных функциях; чистое ядро отделено от I/O; единственная точка
|
||
subprocess (только `git`, cwd = temp-материализация, токен в логе маскируется); секрет в отчёте
|
||
`***` + явный тест non-leak (`test_secret_never_leaks_into_report`); тесты содержательные
|
||
(recording-фейки мутаций, негативные сценарии CE-отказов, AST-проверка импортов, monkeypatch-мины
|
||
на мутации в dry-run) — не тривиальные. Багфикс-трек не применим (задача не `Bug`).
|
||
|
||
### Ось 4 — Документация — ✅ ОБНОВЛЕНА В ТОМ ЖЕ PR
|
||
|
||
| Точка | Статус |
|
||
|---|---|
|
||
| `CLAUDE.md` — раздел «Turnkey-онбординг (ORCH-009)» | ✅ |
|
||
| `docs/architecture/README.md` — раздел + ссылки на ADR | ✅ |
|
||
| `CHANGELOG.md` — запись `feat` (детальная) | ✅ |
|
||
| ADR per-WI `06-adr/ADR-001` + сквозной `adr-0035` + индекс `adr/README.md` | ✅ |
|
||
| `docs/operations/ONBOARDING.md` (новый runbook) | ✅ |
|
||
| `docs/operations/SETUP_WEBHOOKS.md` — обобщён per-repo (ТЗ §2) | ✅ |
|
||
| `onboarding/README.md` — устройство kit, словарь, анти-форк | ✅ |
|
||
| README «Известные ограничения» (ORCH-079) | N/A — онбординг в списке открытых ограничений не значится, обновление не требуется (проверено) |
|
||
| `08-data-requirements.md` отсутствует | Легитимно: гейт `check_analysis_complete` требует только 01–04; ТЗ §5 «изменений БД нет» |
|
||
|
||
## Findings
|
||
|
||
### P0 — Blocker
|
||
Нет.
|
||
|
||
### P1 — Must fix
|
||
Нет.
|
||
|
||
### P2 — Should fix (follow-up, не блокирует)
|
||
- [ ] **Quoted-значение в `.env` → тихая потеря существующих записей в merged-выводе.**
|
||
`read_existing_registry` (fallback-парс `.env`) возвращает значение после `=` как есть; если
|
||
строка в `.env`/`--env-file` взята в кавычки (`ORCH_PROJECTS_JSON='[...]'`), `json.loads`
|
||
в `merged_projects_json` молча даёт `existing=[]` → инструкция оператору содержит массив ТОЛЬКО
|
||
с новым проектом, а runbook §4.1 велит «заменить строку» → риск выпадения enduro/orchestrator
|
||
из реестра. Доминирующий путь безопасен (pydantic `env_file=".env"` снимает кавычки, фоллбек
|
||
срабатывает только при cwd вне корня), потому P2, не P1. Рекомендация: в фоллбеке `strip("'\"")`
|
||
+ предупреждение/GAP, если строка в `.env` есть, а распарсенный existing пуст. (AC-6-периметр;
|
||
ADR D7 «существующие не теряются».)
|
||
- [ ] **`GiteaClient.create_repo`: фоллбек `POST /user/repos` может создать репо в чужом
|
||
namespace.** При `--gitea-owner`, не являющемся ни org, ни юзером токена, отказ org-маршрута
|
||
ведёт в `/user/repos` → репо рождается под юзером токена, последующие webhook/push по
|
||
`owner/repo` дают 404/manual-step. Не деструктивно и видимо в отчёте, но это непрошенная
|
||
мутация не туда. Рекомендация: сверять `owner.login` ответа с запрошенным owner; расхождение →
|
||
`manual-step` (комментарий в коде уже упоминает admin-маршрут — либо реализовать
|
||
`/admin/users/{owner}/repos`, либо честно деградировать).
|
||
- [ ] **CE-деградация Plane + успешный Gitea в одном apply запекает литерал
|
||
`<assigned-on-apply>` в запушенный паспорт.** Если `plane.project` ушёл в `manual-step`, а репо
|
||
создан — kit рендерится с `PLANE_PROJECT_ID="<assigned-on-apply>"` и пушится; повторный apply
|
||
с `--plane-project-id` уже не перезапишет (репо непустой). Скан ловит только `{{…}}`-синтаксис.
|
||
Рекомендация: при неразрешённом PLANE_PROJECT_ID деградировать `kit.materialize`/`kit.push` в
|
||
`manual-step` (push после получения uuid) ИЛИ добавить `<assigned-on-apply>` в скан verify.
|
||
|
||
### P3 — Nice to have
|
||
- [ ] `--env-file` молча игнорируется в `plan`-режиме (`_registry_instructions(report, params,
|
||
None)`): превью merged-массива в plan может расходиться с apply при нестандартном env-файле.
|
||
- [ ] Push-URL с `oauth2:<token>@` остаётся в `.git/config` временного каталога материализации
|
||
после успешного apply (temp-dir не чистится). Рекомендация: cleanup на успехе (на ошибке
|
||
сохранять для дебага, как сейчас).
|
||
|
||
## Документация
|
||
|
||
Обновлена полностью в том же PR (см. таблицу оси 4): паспорт, архитектурный README, CHANGELOG,
|
||
оба ADR + индекс, новый runbook, обобщённый SETUP_WEBHOOKS, README kit. Несоответствий
|
||
«код изменён — дока молчит» не найдено; обзорная витрина README не затронута задачей по
|
||
построению (ограничение в ней не значилось).
|
||
|
||
## Для следующей стадии (testing) — handoff
|
||
|
||
1. **AC-13 (операторский smoke)**: прогон по runbook §5.2 (staging 8501, sandbox `SMK`) должен
|
||
быть выполнен и запротоколирован в «Журнале smoke-прогонов» `ONBOARDING.md`, ссылка — из
|
||
`13-test-report.md` (требование D8). Это единственный непокрытый pytest'ом AC.
|
||
2. Локальный полный регресс гонять с чистой средой (без `ORCH_AGENT_FALLBACK_MODEL`/
|
||
`ORCH_AGENT_EFFORT_*` агент-раннера) — иначе 2 ложных средовых падения в
|
||
`test_resolve_agent_model.py`/`test_resolve_agent_effort.py` (pre-existing, к PR отношения
|
||
не имеют).
|