reviewer(ET): auto-commit from reviewer run_id=751

docs/work-items/ORCH-123/12-review.md

@@ -0,0 +1,153 @@
+---
+verdict: APPROVED
+work_item: ORCH-123
+stage: review
+author_agent: reviewer
+status: approved
+created_at: 2026-06-16
+model_used: claude-opus-4-8
+type: review
+work_item_id: ORCH-123
+version: 1
+---
+
+# Review ORCH-123
+
+## Summary
+
+Фикс инцидента **ORCH-116**: детерминированный staging-runner (ORCH-115) исполнял `docker exec`
+**изнутри** прод-контейнера `orchestrator`, где нет docker CLI (`Dockerfile:11` — только
+`openssh-client git curl`) → `FileNotFoundError` → постоянный environment-дефект ложно
+маршрутизировался как код-фейл-откат `deploy-staging → development` (с расходом developer-retry).
+Задача — `Bug`, эскалирована `escalate: full-cycle` (полный пакет analysis+architecture+ADR).
+
+Изменение точное и минимальное: 9 файлов в реальном скоупе (`65c17d8..HEAD`) —
+`src/staging_runner.py` (+343), `src/config.py` (новый флаг), `src/main.py` (preflight),
+`.env.example`, `CHANGELOG.md`, `CLAUDE.md`, `docs/architecture/README.md`,
+`docs/operations/INFRA.md`, + тесты `tests/test_orch123_staging_runner_exec.py` (503),
+`tests/test_orch115_staging_runner.py` (anti-drift).
+
+Реализация байт-в-байт соответствует ADR-001 / adr-0049 (D1 host-side ssh-wrap, D3 трёхсторонняя
+`classify_staging_outcome`, D4 инвариант «инфра ≠ код-фейл» = infra-HOLD, D5 preflight, D6 флаг,
+D7 сохранность контракта, D8 наблюдаемость). Все четыре оси проверки пройдены; P0/P1 findings нет.
+
+**Проверено фактически:**
+- Полный прогон `pytest tests/ -q` → **2131 passed** (AC-11).
+- Целевой набор `test_orch123_staging_runner_exec.py` + `test_orch115_staging_runner.py` +
+  `test_llm_determinization_docs.py` → **53 passed**.
+- Структурный анти-регресс `test_system_docs.py` + `test_no_host_hardcodes.py` +
+  `test_config.py` → **61 passed**.
+
+## Соответствие ТЗ (02-trz.md / 03-acceptance-criteria.md)
+
+Все функциональные требования и критерии приёмки реализованы и покрыты тестами:
+
+| AC | FR/BR | Статус | Где проверено |
+|----|-------|--------|---------------|
+| AC-1, AC-2 — сюита реально исполняется host-side | FR-1 | ✅ | `build_staging_command` ssh-wrap; TC-02, TC-05 |
+| AC-3 — env/tool-error ≠ код-фейл-откат | FR-2/FR-3 | ✅ | `classify_staging_outcome` + `_handle_permanent_env`/`_handle_transient_infra`; TC-01, TC-03 |
+| AC-4 — анти-over-tolerance (исполнившаяся упавшая сюита → откат) | FR-2/BR-3 | ✅ | check 3 «trust recognised exit-code first»; TC-04, TC-04-exit1-marker |
+| AC-5 — prod-like preflight | FR-4 | ✅ | `preflight()` в `main.lifespan` (best-effort, never-blocks); TC-07 |
+| AC-6 — обязательный регресс red→green | FR-1/FR-2 | ✅ | TC-01 (in-container shape + spawn-error → permanent-env, без advance/defer/retry) |
+| AC-7 — гейт/контракт/стадии/схема без регресса | NFR-1 | ✅ | TC-10, TC-11 (`STAGE_TRANSITIONS`/`QG_CHECKS`/`check_staging_status`/схема БД целы) |
+| AC-8 — self-hosting safety | NFR-2 | ✅ | TC-08 (нет `compose`/`--build`/`8500`/`force`/`push`/`.env`/`restart` в argv) |
+| AC-9 — security-review socket/CLI | NFR-3 | ✅ | ADR D2: выбран host-side ssh, docker CLI/SDK НЕ добавлен, `docker.sock` не используется изнутри |
+| AC-10 — kill-switch/область | NFR-4/NFR-6 | ✅ | TC-09 (`staging_runner_exec_host_side` default True; enduro no-op) |
+| AC-11 — never-raise / очередь не клинится | NFR-5 | ✅ | TC-12; полный suite зелёный |
+| AC-12 — документация границы исполнения | BR-5/FR-6 | ✅ | TC-13 (INFRA.md + README.md содержат host-side/ssh/ORCH-123) |
+
+Регресс R-2 (held `deploy-staging`-задача не откатывается reconciler'ом) — отдельный тест
+`test_r2_held_deploy_staging_not_rolled_back`: при отсутствии `15-staging-log.md` (infra-HOLD)
+`advance_if_gate_passed` отдаёт `None`, задача держится на `deploy-staging`, `advance_stage` не
+зовётся. Закрывает риск реинтродукции дефекта (10-tech-risks R-2).
+
+## Соответствие ADR (06-adr/ADR-001 + сквозной adr-0049)
+
+- **D1** — `build_staging_command` оборачивает ту же `docker exec … staging_check.py … --mode stub`
+  в `ssh -o StrictHostKeyChecking=no <user@host> '<inner>'` (зеркало `self_deploy.build_deploy_command`/
+  `image_freshness.image_revision`); inner-команда `shlex.quote`-ится; fallback на in-container при
+  выключенном флаге ИЛИ пустом ssh-target. ✅
+- **D3** — `classify_staging_outcome` различает три класса с корректным порядком проверок:
+  env-маркер в stderr → `permanent-env` (дизамбигуация `exit=1` коллизии «No such container» vs
+  реальный фейл, R-3); 126/127 → `permanent-env`; распознанный int≠255 без маркера → `suite-ran`
+  (проверяется **до** channel-guards → реальный фейл не маскируется, BR-3); timeout/255 →
+  `transient-infra`; нет ssh-target/`rc=None` → `permanent-env`; неизвестно → fail-safe
+  `transient-infra`. Pure, never-raise. ✅
+- **D4** — инвариант «инфра ≠ код-фейл»: `permanent-env` → немедленный infra-HOLD (без `_advance`,
+  без FAILED-лога, без developer-retry); `transient-infra` → bounded DEFER, на исчерпании — тоже
+  infra-HOLD (суперсед терминала ORCH-115 D5 fail-closed-FAILED+rollback). ✅
+- **D5** — `preflight()` зондирует host-side канал коротким bounded ssh-probe, пишет
+  `_PREFLIGHT_STATE`, алертит; вызван best-effort из `main.lifespan` после lease-recovery,
+  никогда не блокирует старт. ✅
+- **D6** — `staging_runner_exec_host_side: bool = True` (env `ORCH_STAGING_RUNNER_EXEC_HOST_SIDE`),
+  дефолт = боевое; откат двумя флагами. ✅
+- **D7** — `STAGE_TRANSITIONS`/`QG_CHECKS`/`check_staging_status`/`_parse_staging_status`/
+  machine-verdict `staging_status:`/схема БД — байт-в-байт (TC-10/TC-11). ✅
+- **D8** — `snapshot()` различает три не-успешных класса (`failed`=code-fail / `deferred`=transient /
+  `permanent_env`=infra-HOLD) + `exec_host_side` + `preflight`. ✅
+
+**Трассировка маркеров (CLAUDE.md §9 / TRACEABILITY.md):** ORCH-123 правит блоки ORCH-115
+(staging-runner, прямой объект), ORCH-036 (self_deploy ssh-паттерн), ORCH-110 (proc_group/classify
+/infra-tolerance), ORCH-058 (host-side docker). ADR явно сверяется с их инвариантами и не ломает
+их; анти-дрейф ORCH-115 — TC-14 (`should_intercept`/`map_exit_code_to_status` shared-контракт,
+kill-switch → LLM) зелёный.
+
+## Качество кода
+
+- Чистый leaf-стиль по образцу `merge_gate`/`self_deploy`/`image_freshness`; на импорте только
+  `config`/`shlex`/`subprocess`, тяжёлое — лениво. Все публичные функции с docstrings.
+- `never-raise` соблюдён на всех путях (classify, preflight, snapshot, run_staging_gate, alert'ы).
+- Старое имя `_handle_tool_error` чисто переименовано в `_handle_transient_infra` — висячих ссылок
+  нет (grep пуст).
+- Тесты содержательные (поведенческие, не тривиальные): мокается subprocess/advance/notifications,
+  spawn-error воспроизводится несуществующим бинарём — без живого ssh/docker/сети.
+- **Багфикс-трек (ORCH-019 BR-4):** обязательный регресс-тест-фиксатор дефекта присутствует —
+  TC-01 красный на коде до фикса (нет класса/счётчика `permanent_env`, путь advance'ит + DEFER),
+  зелёный после. Требование выполнено.
+
+## Документация
+
+`src/` изменён → документация обновлена **в том же PR** (golden source):
+- `docs/operations/INFRA.md` — новый раздел «Граница исполнения docker-операций — host-side» +
+  правило 4 для агентов (FR-6 / AC-12). ✅
+- `docs/architecture/README.md` — секция Staging-runner дополнена host-side стратегией +
+  трёхсторонней классификацией + ссылкой на adr-0049. ✅
+- `CLAUDE.md` — раздел ORCH-123 (host-side exec + классификация environment-дефекта). ✅
+- `CHANGELOG.md` — запись `fix(staging)`. ✅
+- `.env.example` — новый ключ `ORCH_STAGING_RUNNER_EXEC_HOST_SIDE=true` + описание. ✅
+- ADR: `06-adr/ADR-001-…` + сквозной `docs/architecture/adr/adr-0049-…`. ✅
+
+**Витрина системы `docs/overview/` (ORCH-011)** — обновление НЕ требуется: канал исполнения
+(host-side vs in-container) — внутренняя деталь реализации; машинно-проверяемые факты витрины
+(детерминированный раннер заменил LLM-`deployer`, kill-switch, скоуп) ORCH-123 не меняет;
+`test_system_docs.py` зелёный.
+
+**Обзорные доки `README.md` «Известные ограничения» (ORCH-079)** — обновление НЕ требуется:
+ORCH-123 закрывает внутренний регресс ORCH-116, а не задокументированное ограничение; релевантного
+пункта в списке нет.
+
+## Findings
+
+### P0 — Blocker
+- Нет.
+
+### P1 — Must fix
+- Нет.
+
+### P2 — Should fix
+- Нет.
+
+### P3 — Nice-to-have (не блокирует)
+- `preflight()` использует прямой `subprocess.run(timeout=20)` вместо `proc_group` — для короткого
+  read-only ssh-зонда приемлемо (тот же паттерн, что у host-side probe'ов `image_freshness`);
+  при таймауте локальный ssh-клиент завершается subprocess'ом без tree-kill, что для 20-секундного
+  inspect незначимо. Можно унифицировать в будущем, не обязательно.
+- `_channel_ssh_configured()` вызывается дважды в `run_staging_gate` (для classify и для лог-строки
+  `permanent-env`) — микро-избыточность, never-raise, влияния нет.
+
+## Вердикт
+
+**APPROVED.** Реализация полно и точно покрывает ТЗ/AC, соответствует ADR-001/adr-0049, не трогает
+гейты/стадии/контракт/схему БД, безопасна для self-hosting, документация (включая витрину-проверку
+и обзорные доки) в порядке, обязательный багфикс-регресс присутствует и зелёный, полный suite
+(2131) зелёный. P0/P1 отсутствуют.