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

verdict, work_item, stage, author_agent, status, created_at, model_used, type, work_item_id, version

verdict	work_item	stage	author_agent	status	created_at	model_used	type	work_item_id	version
APPROVED	ORCH-123	review	reviewer	approved	2026-06-16	claude-opus-4-8	review	ORCH-123	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 отсутствуют.