reviewer(ET): auto-commit from reviewer run_id=605
All checks were successful
CI / test (push) Successful in 1m1s
CI / test (pull_request) Successful in 59s

This commit is contained in:
2026-06-10 20:57:20 +03:00
parent f1635ddb39
commit 26fe4cdd5e

View File

@@ -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`, D1D10): нормативный реестр хардкодов §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 инфра-файлы | ✅ | реестр B1B6/C1C2/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 по D1D10. Правки блоков с чужими маркерами сверены с их 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 (детальная, по слоям D1D10) |
| `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-промпта.