diff --git a/docs/work-items/ORCH-101/12-review.md b/docs/work-items/ORCH-101/12-review.md new file mode 100644 index 0000000..3303cd6 --- /dev/null +++ b/docs/work-items/ORCH-101/12-review.md @@ -0,0 +1,118 @@ +--- +verdict: APPROVED +work_item: ORCH-101 +stage: review +author_agent: reviewer +status: approved +created_at: 2026-06-10 +model_used: claude-opus-4-8 +type: review +work_item_id: ORCH-101 +version: 1 +--- + +# Review ORCH-101 — ORCH-10-common: расхардкод + секреты + smoke (фундамент тиража) + +## Summary + +PR (`feature/ORCH-101-orch-10-common-smoke`, commit `f1635dd`) реализует фундамент тиража +ровно по ТЗ (`02-trz.md`) и ADR (`06-adr/ADR-001`, D1–D10): нормативный реестр хардкодов §3.1 +закрыт полностью, секреты нового хоста выпускаются `scripts/gen_secrets.py`, smoke-процедура — +runbook `docs/operations/REPLICATION.md` с явными PASS/FAIL. Полный `pytest tests/ -q` зелёный +(**1764 passed**), включая 6 новых/адаптированных тест-модулей. P0/P1 findings — нет; +два косметических P3. Вердикт: **APPROVED**. + +### Ось 1 — Соответствие ТЗ (FR-1…FR-7, AC-1…AC-8) + +| AC | Статус | Проверено | +|----|--------|-----------| +| AC-1 ноль хардкодов в `src/**` | ✅ | A1 `plane_sync.notify_stage_change` → `gitea_public_url or gitea_url` + `gitea_owner` (семантика существующих потребителей, литералы `git.mva154.duckdns.org`/`/admin/` удалены); A2 — единый `launcher.agent_git_env()` для ОБОИХ мест (Popen агента + post-run git); A3/A4 — `self_deploy`/`post_deploy` читают `agent_home_dir`/`git_email_domain`, платформенные имена `deploy-finalizer`/`post-deploy-monitor` сохранены (D2); A5 → ключ `staging_port` + guard (D4); A6 → платформенная константа c обоснованием в ADR D3 и пин-тестом (`test_self_hosting_repo_is_platform_constant` фиксирует и значение, и ОТСУТСТВИЕ конфиг-ключа); A7 — основной путь уже config-backed, except-ветка запрещённых литералов не содержит (ADR D10). Контрольный grep: остаточные `mva154`/`/home/slin` в `src/**`/`watchdog/**` — только комментарии/докстринги. Сканер существует, allowlist пуст, зелёный | +| AC-2 zero-regression | ✅ | дефолты новых ключей = боевым (`test_new_settings_defaults_equal_previous_production_values`, судит по `Settings.model_fields` — иммунно к ambient env); реестр E не тронут (там же); резолв compose при пустом env = боевым значениям (TC-06, `EXPECTED_DEFAULTS` — единая нормативная карта); `pytest tests/ -q` → 1764 passed; `STAGE_TRANSITIONS`/`QG_CHECKS`/`check_*`/verdict-ключи/схема БД дифф не трогает | +| AC-3 smoke-процедура | ✅* | `REPLICATION.md` §4: 7 шагов, каждый с явными PASS/FAIL, однозначный итоговый вердикт; stateless (§5); кирпичи существующие (D9, скрипт-обвязка сознательно не вводилась). *Прогон воспроизводимости — стадия `testing` (см. «Передача тестеру») | +| AC-4 доки + CHANGELOG | ✅ | см. раздел «Документация» ниже | +| AC-5 секреты | ✅ | `secrets.token_hex(32)` = 64 hex (тест `test_secret_is_cryptorandom_64_hex` + неравенство повторных запусков); `--write` через `open(path, "x")` — атомарный отказ exit=2 на существующем файле, перезапись только `--force` (NFR-3, покрыто тестами); чек-лист внешних токенов §3.2 (где выпустить → куда вписать → как проверить) + норматив «боевые секреты не копируются»; `.env.example` дополнен (полнота сверяется `test_env_example_contains_*` и `test_output_keys_consistent_with_env_example`); реальных секретов в гите нет (плейсхолдеры пустые, анти-тест `test_no_real_secret_committed_anywhere_near`) | +| AC-6 инфра-файлы | ✅ | реестр B1–B6/C1–C2/D1 закрыт: compose — полная интерполяция `${VAR:-default}` (карта D6 1:1); Dockerfile — `ARG APP_UID/GID/USER/HOME`, CMD exec-form 8500 сознательно сохранён (D5, PID-1/сигнальная семантика); hook — `REPO="${REPO:-…}"` + ОБА инвокера передают `REPO=` явно (D7, иначе override был бы мёртв — закрыто `test_rebuild_staging_passes_configured_port_and_repo`/`test_build_deploy_command_passes_repo_explicitly`) | +| AC-7 анти-регресс | ✅ | `test_no_host_hardcodes.py`: централизованный `FORBIDDEN`, `tokenize`-исключение комментариев/докстрингов (NFR-5, детерминирован), структурное исключение config-модулей по ADR D10 (не allowlist-запись), allowlist пуст + тест на пустоту, негативные самопроверки ×4 (plain/env-dict/f-string/значение при докстринге выше) + guard от пустой зоны скана (`test_scan_zone_is_nonempty`) | +| AC-8 self-hosting / инварианты | ✅ | прод-контейнер не трогается (изменения compose/Dockerfile вступают в силу на следующем штатном деплое); ORCH-058 **усилен**: fail-closed guard `staging_port == deploy_prod_target_port` → отказ ДО любого ssh/build, без тихого fallback, + Telegram-алерт (тесты: guard срабатывает на 8500/8500 и НЕ срабатывает на дефолте 8501/8500); INV-4 не затронут; маркеры соседей сохранены (ось 2) | + +### Ось 2 — Соответствие ADR + трассировка (TRACEABILITY) + +Реализация 1:1 по D1–D10. Правки блоков с чужими маркерами сверены с их ADR: + +- **ORCH-040** (adr-0005): `group_add` «МИНА 1» сохранён на всех трёх сервисах (только литерал → + `${ORCH_DOCKER_GID:-999}`); uid/gid/HOME/маунт-таргеты двигаются одной группой env + (`user:` + `build.args APP_*` + таргеты `.claude`/`.ssh` — все от `ORCH_RUN_UID/GID`/ + `ORCH_AGENT_HOME_DIR`); адаптация `test_orch040_compose.py` корректна — судит резолв дефолтов, + инвариант НЕ ослаблен (смена дефолта по-прежнему валит тест). +- **ORCH-058** (adr-0008): explicit-pass дисциплина сохранена (`TARGET_PORT=` по-прежнему передаётся + явно, источник — валидируемый ключ); анти-prod инвариант из подразумеваемой константы стал + исполняемым guard'ом — усиление, не ослабление; имена сервиса/профиля остались константами. +- **ORCH-036** (adr-0007): exit-code контракт хука 0/1/2 не тронут — в префикс добавлено ровно одно + env-присваивание `REPO=`; detached Phase B не изменён. +- **INV-4**: дифф не содержит push/force-push в `main`; merge-путь не тронут. +- Сквозной `adr-0036` заведён (кросс-каттинг по TRACEABILITY — корректно, блоки несут 3 маркера). + +### Ось 3 — Качество кода + +- `agent_git_env()` устраняет дублирование ×2 в launcher (DRY) и структурно исключает дрейф двух + мест; docstrings на всех новых публичных функциях (`agent_git_env`, `generate_secret`, + `build_fragment`, `main`). +- Guard в `check_staging_image_fresh` размещён правильно: после дешёвых kill-switch/`applies` + (нулевой оверхед для неприменимых репо), ДО любого ssh/build; Telegram-алерт best-effort + (`try/except`, never-raise сохранён). +- Тесты содержательные: функциональные через monkeypatch-швы (link-build без сети, guard, передача + `REPO=`/`TARGET_PORT=` в собранную команду), структурные пины там, где env-словарь строится inline + в never-raise акторе — осознанный выбор, задокументирован в докстринге модуля. Негативные + самопроверки исключают «вечно-зелёный» сканер. `test_deploy_hook_rollback_sim.py` переведён на + env-override `REPO` — симуляция теперь дополнительно доказывает работоспособность override. +- Security: секретные значения в `.env.example` — пустые плейсхолдеры; генератор не несёт зашитых + hex-значений (анти-тест); `open(..., "x")` — race-safe отказ от перезаписи. + +### Ось 4 — Документация + +Полностью обновлена в том же PR, см. раздел «Документация» ниже. README «Известные ограничения» +(ORCH-079): ни один из трёх открытых пунктов (Telegram 48h / intra-repo deps / пакетный автоном) +задачей не закрывается — обновление витрины не требуется; таблица env README дополнена. + +## Findings + +### P0 — Blocker +— нет. + +### P1 — Must fix +— нет. + +### P2 — Should fix +— нет. + +### P3 — Nice to have (не блокирует, на усмотрение следующих задач) +- [ ] `src/plane_sync.py::notify_stage_change`: при пустых ОБОИХ `gitea_public_url`/`gitea_url` + (нештатная конфигурация) собирается некорректная Markdown-ссылка вида `(/owner/...)`. Покрыто + never-raise конвертом и нереально при валидном конфиге (у `gitea_url` непустой дефолт); + опциональный guard «пустая база → пропустить ссылку» — косметика. Правило: ТЗ FR-1/A1 требует + только источник из Settings — выполнено. +- [ ] `tests/test_no_host_hardcodes.py::find_violations`: эвристика statement-position пропускает + любой bare-string-statement (не только докстринги) — теоретический класс ложноотрицательных + (литерал в строке-выражении без присваивания). Реалистичные паттерны регрессии (env-dict, + f-string, RHS-значение) закрыты негативными самопроверками; ужесточение — отдельной задачей при + желании. Правило: NFR-5/AC-7 — выполнены как сформулированы. + +## Документация + +| Артефакт | Статус | +|----------|--------| +| `CHANGELOG.md` | ✅ запись ORCH-101 (детальная, по слоям D1–D10) | +| `CLAUDE.md` | ✅ новая секция «Тираж платформы: фундамент 10-common» | +| `README.md` | ✅ таблица env (+6 строк ORCH-101) + ссылка на REPLICATION.md; «Известные ограничения» не затронуты (корректно) | +| `docs/architecture/README.md` | ✅ секция ORCH-101 со ссылками на adr-0036/work-item ADR | +| `docs/operations/INFRA.md` | ✅ карта env дополнена (7 строк) + норматив секретов + когерентность портов | +| `docs/operations/REPLICATION.md` | ✅ новый runbook: границы/конвенции, карта env, секреты, smoke PASS/FAIL, stateless | +| ADR per work-item + сквозной | ✅ `06-adr/ADR-001-…` + `adr-0036-…` (frontmatter-схема 52c корректна) | +| `.env.example` / `.env.staging.example` | ✅ полнота ключей старта (закреплена тестами) / согласован | + +## Передача тестеру (не findings) + +- **AC-3 (воспроизводимость):** заявленный прогон smoke-процедуры на текущей инфре + (staging 8501 + sandbox-проект) исполняется на стадии `testing` и фиксируется в + `13-test-report.md`/`15-staging-log.md` — без этой фиксации AC-3 не считается закрытым. +- Smoke `serial_gate`/`GET /queue` — штатно по канону tester-промпта.