From cc3ed42041768c88e711506f6a09a8d6b91da76d Mon Sep 17 00:00:00 2001 From: claude-bot Date: Wed, 10 Jun 2026 15:55:43 +0300 Subject: [PATCH] reviewer(ET): auto-commit from reviewer run_id=589 --- docs/work-items/ORCH-009/12-review.md | 145 ++++++++++++++++++++++++++ 1 file changed, 145 insertions(+) create mode 100644 docs/work-items/ORCH-009/12-review.md diff --git a/docs/work-items/ORCH-009/12-review.md b/docs/work-items/ORCH-009/12-review.md new file mode 100644 index 0000000..93b8107 --- /dev/null +++ b/docs/work-items/ORCH-009/12-review.md @@ -0,0 +1,145 @@ +--- +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-секций в нормативном порядке, «❌→✅», `` у dev/reviewer/tester, `` — паритет с эталоном (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 запекает литерал + `` в запушенный паспорт.** Если `plane.project` ушёл в `manual-step`, а репо + создан — kit рендерится с `PLANE_PROJECT_ID=""` и пушится; повторный apply + с `--plane-project-id` уже не перезапишет (репо непустой). Скан ловит только `{{…}}`-синтаксис. + Рекомендация: при неразрешённом PLANE_PROJECT_ID деградировать `kit.materialize`/`kit.push` в + `manual-step` (push после получения uuid) ИЛИ добавить `` в скан verify. + +### P3 — Nice to have +- [ ] `--env-file` молча игнорируется в `plan`-режиме (`_registry_instructions(report, params, + None)`): превью merged-массива в plan может расходиться с apply при нестандартном env-файле. +- [ ] Push-URL с `oauth2:@` остаётся в `.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 отношения + не имеют).