From 93c1df9f72de2ef1fdd9e5059888c7887fd8ceae Mon Sep 17 00:00:00 2001 From: claude-bot Date: Fri, 12 Jun 2026 03:13:01 +0300 Subject: [PATCH] reviewer(ET): auto-commit from reviewer run_id=644 --- docs/work-items/ORCH-104/12-review.md | 146 ++++++++++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100644 docs/work-items/ORCH-104/12-review.md diff --git a/docs/work-items/ORCH-104/12-review.md b/docs/work-items/ORCH-104/12-review.md new file mode 100644 index 0000000..06ce462 --- /dev/null +++ b/docs/work-items/ORCH-104/12-review.md @@ -0,0 +1,146 @@ +--- +verdict: REQUEST_CHANGES +work_item: ORCH-104 +stage: review +author_agent: reviewer +status: changes-requested +created_at: 2026-06-12 +model_used: claude-opus-4-8 +type: review +work_item_id: ORCH-104 +version: 1 +--- + +# Review ORCH-104 — Установочный скрипт Lite-тиража (`scripts/setup_lite.py`) + +## Summary + +Задача — **scripts + docs + tests**: исполняемый интерактивный installer Lite-тиража поверх +канона `LITE_SETUP.md`. Качество **чистых функций** и **документации** — высокое: рантайм +`src/**`/`docker-compose.yml`/`Dockerfile`/`.env*.example` байт-в-байт не тронуты (AC-13 ✓), +оба тест-файла зелёные (74 passed), `LITE_SETUP.md` §1.1 + footer-норматив + CHANGELOG + +витрина `docs/overview/` + `docs/architecture/README.md` + ADR (локальный и сквозной +`adr-0040`) обновлены в том же PR (AC-14 ✓, документация — golden source соблюдён). + +**Однако** обнаружен блокер: **решающая логика реализована как чистые функции, но НЕ +подключена к исполняемому пути `apply`** — `step_collect` пустой, `io.ask(...)` не вызывается +ни разу, результаты discovery и флаги `--project-*` не попадают в `answers`, машинные guard-ы +(C-1 Telegram, §6.4 branch protection, когерентность портов, Path Б webhook) не вызываются из +шагов. Реальный прогон `apply` НЕ собирает ключи и НЕ выполняет установку, ради которой задача +заведена. Это «не реализовано требование ТЗ» (FR-1/FR-3/FR-4/FR-5/FR-7/FR-9) и нарушение +собственного ADR D3/D5/D9 → `REQUEST_CHANGES`. + +Анти-доказательство «доки точны»: §1.1 LITE_SETUP и CHANGELOG утверждают, что скрипт +«запрашивает обязательные ключи в момент установки с немедленной верификацией» — исполняемый +код этого не делает (документация описывает спецификацию, а не текущую реализацию). + +## Findings + +### P0 — Blocker + +- [ ] **Интерактивный сбор ключей не подключён — `apply` не функционален end-to-end.** + `step_collect` (`scripts/setup_lite.py:1061-1066`) — пустая заглушка: тело делает только + `ctx.setdefault("answers", {})` и `return "ok"`, хотя docstring обещает «значения собираются + в ctx['answers']». Метод `IO.ask` (`:181`) с верификацией/env-prefill/лимитом попыток + реализован и протестирован (TC-10…12), но в самом скрипте `io.ask(...)` **не вызывается + нигде** (подтверждено grep). Следствие: при реальном `apply` ни один из + `MANDATORY_NEW_HOST_KEYS` (Plane URL/токен, Gitea, Telegram, хост-параметры, порты) не + собирается → `step_render_env` рендерит `.env`/`.env.watchdog` только со свежими + webhook-секретами поверх **плейсхолдеров** `.env.example`, а `build_onboard_args` получает + пустые `--name/--repo/--prefix`. Нарушены **FR-4** (`02-trz.md` §3, «немедленная верификация + каждого введённого значения») и **ADR D3** (шаг 4 `collect` = «интерактивный сбор ключей с + немедленной верификацией»). Headless-путь тоже мёртв: env-prefill живёт внутри `ask`, который + не зовётся. + +- [ ] **Результаты discovery и `--project-*` не доходят до `answers` (FR-3/FR-9 не + реализованы).** `step_discovery` пишет `ctx["chosen_plane"]/["chosen_gitea"]` + (`:1056-1057`), но эти значения **нигде не читаются** — кандидаты URL вычисляются и + отбрасываются (FR-3 «префилл кандидатов URL» не реализован). Флаги `--project-name/-repo/ + -prefix/...` (`:1264-1271`) парсятся, но **не копируются** в `ctx["answers"]`; + `build_onboard_args` (`:722-742`) читает `answers.get("project_name")` и т.д., которые + всегда пусты → `onboard_project.py` вызывается с пустыми параметрами проекта (FR-9 / AC-10 + в исполняемом пути не выполняется). + +- [ ] **Машинные guard-ы ТЗ не вызываются из шагов (FR-5/FR-7 не реализованы end-to-end).** + Подтверждено grep: `telegram_c1_verdict` (C-1 ORCH-100), `port_overrides` (когерентная + тройка), `staging_port_ok` (staging≠prod fail-closed), `next_free_port` (busy-check + альтернатива) — **не вызываются** нигде в скрипте. `step_gitea_guards` (`:1103-1115`) читает + `ctx.get("gitea_bp_status")`/`ctx.get("gitea_branch_protections")`, которые **никогда не + устанавливаются** → §6.4 branch-protection guard всегда возвращает `"ok"` (никогда не + срабатывает). `step_plane_webhook` (`:1091-1100`) уходит в Path Б только при `ctx.get("psql")` + и `answers.get("plane_db_container")` — оба **никогда не устанавливаются** → Path Б + физически недостижим. Нарушены **FR-5** (`02-trz.md` §3), **FR-7** (C-1, §6.4) и **ADR D9**: + guard-ы заявлены «машинными», но в рантайме мертвы. + +### P1 — Must fix + +- [ ] **Контракт идемпотентности/resume (check→ensure) не реализован на уровне движка.** Все + `APPLY_STEPS` используют `_always_run` (`:998-999`, реестр `:1214-1225`), который всегда + возвращает `False` → ни один продакшн-шаг никогда не даёт `"skip"`. Заявленный **AC-1** + («на уже выполненном хосте шаги дают каскад skip; повторный запуск не перевыполняет + мутации») и **ADR D3** («каждый шаг сперва проверяет „уже сделано?“ … при PASS пропускается») + не выполнены: повторный `apply` заново гоняет `gen_secrets`, `docker compose up -d --build` + и `onboard_project.py plan/apply/verify`. Сам `run_steps` skip поддерживает (TC-02/03 на + фейковом check), но реальные шаги его не используют. + +- [ ] **Нет интеграционного теста `run_apply` — зелёный suite маскирует P0.** + `tests/test_setup_lite_script.py` тщательно покрывает чистые функции **в изоляции** (вердикты, + discovery, `port_overrides`, `build_onboard_args`, exit-контракт), но **ни один тест не + проверяет, что `run_apply`/`step_collect` действительно собирают ключи и протягивают их в + `answers`/render/onboard**. Именно поэтому полностью оторванная проводка (P0) проходит при + 74 passed. Ось «тесты содержательные»: нужен сценарный тест шага collect → render → onboard + на инжектируемом I/O (инфраструктура `IO` для этого уже есть, D10). + +### P2 — Should fix + +- [ ] **Порт health-чека захардкожен `DEFAULT_PROD_PORT` (8500), игнорирует override + прод-порта.** `step_up` (`:1134` `port = DEFAULT_PROD_PORT`) и `run_verify` (`:1337`) бьют + в 8500 независимо от выбранного прод-порта (FR-5). После починки сбора порта это даст ложный + FAIL health на не-дефолтном порте; брать порт из собранного `ORCH_DEPLOY_PROD_TARGET_PORT`. + +- [ ] **`url` подставляется в SQL без валидации/экранирования (расхождение с ADR D8).** + `build_webhook_insert_sql` (`:607-619`) и `count_sql` (`:630-631`) интерполируют + `answers["orchestrator_public_url"]` в текст SQL напрямую; ADR D8 заявляет «slug и прочие + подстановки валидируются … анти-SQL-инъекция на пользовательском вводе». Импакт низкий (своя + БД заказчика, путь Path Б сейчас недостижим), но при включении Path Б — валидировать `url` + (как `valid_slug`) или передавать psql-переменной. + +- [ ] **`run_verify` вызывает `stateless_verdict(queue)` без `own_prefixes` (`:1347`)** → после + онбординга проекта заказчика его собственные задачи в `/queue` будут помечены «чужими» и + `verify` даст ложный FAIL. Протянуть префикс проекта из конфигурации/реестра. + +## Документация + +**Статус: обновлена в том же PR (ось документации — PASS).** +- `docs/deployment/LITE_SETUP.md` — добавлен `### 1.1. Быстрый путь: setup_lite.py` (пиннинг + «13 разделов в порядке» цел, `test_lite_setup_doc.py` зелёный) + footer-норматив сопровождения + расширен «обнови док **и** `scripts/setup_lite.py` в том же PR» (FR-11/D12 ✓). +- `CHANGELOG.md` — запись `feat:` (ORCH-104) есть. +- Витрина системы `docs/overview/README.md` (маршрут «Развернуть у себя») и + `docs/architecture/README.md` (блок Type A) дополнены — правило агентов №2 / ORCH-011 ✓. +- Паспорт `CLAUDE.md` — секция ORCH-104 добавлена; ADR `06-adr/ADR-001-…` + сквозной + `adr-0040-lite-interactive-installer.md` присутствуют. + +> Замечание (учесть при доработке, НЕ блокер само по себе): после починки P0 проверить, что +> §1.1 / CHANGELOG не «опережают» реализацию (сейчас формулировка «запрашивает обязательные +> ключи … с немедленной верификацией» соответствует ТЗ, но не текущему коду). Доводка кода до +> спецификации это снимает. + +## Что проверено и в порядке + +- **AC-13 (рантайм байт-в-байт):** `git diff origin/main...HEAD` по `src/**`, + `docker-compose.yml`, `Dockerfile`, `.env.example`, `.env.watchdog.example` — пусто. ✓ +- **Гигиена скрипта (AC-12):** stdlib-only, без `src.*`, no-delete, секреты через `getpass` и + не печатаются, кирпичи `gen_secrets.py`/`onboard_project.py` — субпроцессами; структурные + тесты TC-24/25 зелёные. ✓ (замечание: гигиена соблюдена, но функциональная проводка — нет). +- **Тесты:** `pytest tests/test_setup_lite_script.py tests/test_lite_setup_doc.py -q` → + 74 passed. ✓ (зелёные, но без интеграционного покрытия — см. P1). + +## Вывод + +Чистые функции, документация и инварианты рантайма — на эталонном уровне. Но исполняемый +`apply` не делает того, ради чего задача заведена: интерактивный сбор/верификация ключей, +префилл из discovery, протяжка параметров проекта и срабатывание машинных guard-ов не +подключены к step-движку. Это P0 → **REQUEST_CHANGES**. После подключения чистых функций к +`step_collect`/`step_discovery`/`step_*-guards` (и добавления интеграционного теста `run_apply`) +артефакт будет соответствовать ТЗ и ADR.