reviewer(ET): auto-commit from reviewer run_id=644
This commit is contained in:
146
docs/work-items/ORCH-104/12-review.md
Normal file
146
docs/work-items/ORCH-104/12-review.md
Normal file
@@ -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.
|
||||
Reference in New Issue
Block a user