orchestrator/docs/work-items/ORCH-116/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-116	review	reviewer	approved	2026-06-16	claude-opus-4-8	review	ORCH-116	3

Review ORCH-116 — детерминированный test-раннер вместо LLM-тестера

Summary

PR заменяет LLM-агента tester на стадии testing (self-hosting orchestrator) детерминированным leaf src/test_runner.py, перехватываемым в launch_job до _spawn — точное зеркало уже работающего в проде ORCH-115/staging_runner. Ревью выполнено независимо по всем четырём осям (ТЗ/AC · ADR · качество кода · документация) с личным прогоном тестов и сверкой сигнатур зависимостей. Критический инвариант NFR-1 («замена продюсера артефакта, не гейта») соблюдён байт-в-байт: src/stages.py, src/qg/checks.py, src/staging_runner.py, src/transition_lease.py, src/proc_group.py — не тронуты (подтверждено git diff --stat); в src/ изменены только test_runner.py (новый), launcher.py/config.py/main.py (аддитивно). Документация = golden source обновлена в том же PR (8 doc-файлов + CHANGELOG + ADR-001/adr-0049). Полный регресс проверен лично: pytest tests/ -q → 2137 passed; ORCH-116 + LLM-анти-дрейф + system-docs → 74 passed. P0/P1/P2 findings отсутствуют → APPROVED.

Оси проверки

1. Соответствие ТЗ / Acceptance Criteria — PASS

Верифицировано по коду + тестам + личному прогону каждое AC:

• AC-1 (перехват до _spawn) — launcher.launch_job: if job.get("agent")=="tester": → if test_runner.should_intercept(job): return self._run_test_runner_job(job) вставлено перед return self._spawn(...); _run_test_runner_job ведёт jobs через mark_job(done|failed), возвращает None (нет agent_runs). TC-05 воспроизводит без живого CLI (spawn.assert_not_called).
• AC-2 (контракт 13-test-report.md) — build_test_report эмитит result: PASS|FAIL UPPERCASE
  • полную 52c-схему; читается неизменённым _parse_tests_verdict (TC-04, лично подтверждено прогоном через реальный парсер).
• AC-3 (exit→verdict) — map_exit_code_to_result — тонкий транслятор над единым self_deploy.map_exit_code_to_status (0→PASS, иначе/None/нечисло→FAIL, fail-closed); второй маппинг не введён (TC-02 сверяет с базовым контрактом).
• AC-4 (эквивалентность маршрутизации) — _advance(current_stage="testing", finished_agent="tester"), без новых рёбер; PASS→advance, FAIL→существующий откат testing→development (TC-07 параметризован [PASS/FAIL]).
• AC-5 (two-level outcome, анти-ORCH-110) — suite_ran = (returncode is not None) and (not timed_out); tool-error → bounded DEFER re-queue tester-джоба (TC-10 проверяет agent == "tester"), не код-фейл-откат, не жжёт developer-retry; исчерпание бюджета → fail-closed FAIL + alert.
• AC-6 (анти-дрейф) — git diff по src/ затрагивает только launcher.py/config.py/main.py/test_runner.py; src/stages.py, src/qg/checks.py байт-в-байт не тронуты (проверено git diff --stat лично); новых таблиц нет (TC-09).
• AC-7/AC-8 (kill-switch / backward-compat) — applies() гасит при enabled=False, скоупит CSV; _has_test_contract → False для не-self репо даже при ручном добавлении в CSV (TC-01/TC-08).
• AC-9 (never-raise) — все публичные функции в try/except; run_test_gate имеет внешний guard; _run_test_runner_job дополнительно оборачивает (TC-11: воркер не падает / не клинит).
• AC-10 (self-hosting safety) — build_test_command без литералов рестарта/compose/build/force-push/.env; smoke строго read-only GET; лог пушится только в фичеветку, никакой прямой работы с main (TC-13).
• AC-11 (proc_group/worktree/timeout) — pytest в get_worktree_path(repo, branch) через run_in_process_group (tree-kill), _resolve_timeout дефолтит на малформе + WARNING; без правки reaper_max_running_s (TC-12).
• AC-12/AC-13/AC-14/AC-15 — гибрид off-control-path, блок test_runner в /queue, обновление LLM-карты/политики/roadmap/витрины, полный регресс — все зелёные (AC-15: 2137 passed, проверено лично).

2. Соответствие ADR — PASS

Реализация дословно следует ADR-001 (D1–D12) и сквозному adr-0049. Сверено критическое:

• D6.1 (анти-коллизия 52c-status: ↔ _parse_tests_verdict) — самая тонкая мина задачи. Сверено по неизменённому парсеру (src/qg/checks.py:226-282): он читает вердикт из ТРЁХ равноранговых полей verdict:/status:/result: c negative-token-priority. build_test_report жёстко выравнивает sub_status = "success" if result=="PASS" else "failed": для PASS конкатенация "SUCCESS PASS" не содержит ни одного из _TESTS_NEGATIVE_TOKENS (BLOCKED/FAILED/FAIL/REQUEST_CHANGES/REJECT/RED), позитив PASS из result: → True; для FAIL "FAILED FAIL" → негативный токен → False. Прибито TC-04 (test_tc04_status_field_never_false_negatives_a_pass) через реальный парсер. Мина снята верно.
• D5 DEFER — зеркало staging_runner._handle_tool_error, но re-queue tester-джоба (не deployer!), счётчик restart-safe по маркеру test-runner infra-retry в task_content (без правки схемы БД). Проверено TC-10.
• D4 единый маппинг — переиспользован self_deploy.map_exit_code_to_status (BR-4), второй несогласованный маппинг не введён.
• Трассировка маркеров (TRACEABILITY): правки рядом с ORCH-115 (launcher врезка), ORCH-114 (transition-lease берётся внутри advance_stage — раннер не трогает, граница O1), ORCH-110 (proc_group переиспользуется как есть) — ни один зафиксированный инвариант не сломан.
• Глобальные ADR (INV-4 «мерж только через PR», запрет рестарта прод-контейнера) — соблюдены: лог пушится только в фичеветку (git push origin <branch>, не force, не main); смоук read-only.

3. Качество кода — PASS

• Docstrings на всех публичных функциях; контракт never-raise выдержан сквозно (каждый внешний вызов обёрнут; счётчики через _bump не ломают решение).
• Сигнатуры зависимостей сверены лично (риск latent false-FAIL устранён): db.enqueue_job(agent, repo, task_content, task_id=, available_at_delay_s=), db.mark_job, self_deploy.map_exit_code_to_status, git_worktree.get_worktree_path(repo, branch), qg.checks.is_self_hosting_repo, config.post_deploy_base_url (дефолт http://localhost:8500), notifications.send_telegram/link_for, proc_group.run_in_process_group/ProcResult — все существуют ровно как используются.
• Тесты содержательные (TC-01…TC-15, ~32 тест-функции, не тривиальные): kill-switch/scope/contract, маппинг по классам входа, рендер+неизменённый парсер, перехват до _spawn, two-level outcome, never-raise, timeout-resolve, smoke read-only (включая транзиент-ретрай и missing-serial_gate), анти-дрейф конвейера, наблюдаемость, структурный лог.
• Security: smoke — только urllib GET к локальному оркестратору; мутирующих запросов нет.
• Багфикс-трек неприменим: это не Bug-задача (full-cycle, стадия architecture отработана, есть ADR) → требование регресс-фиксатора ORCH-019/BR-4 не применяется; покрытие тем не менее исчерпывающее.

4. Документация — PASS (обязательная проверка)

src/ изменён → документация обновлена в том же PR, проверено явно по git diff (наличие правок подтверждено по каждому файлу):

• Паспорт CLAUDE.md — новый раздел «Детерминированный test-раннер вместо LLM-тестера (ORCH-116)».
• Компонент-карта docs/architecture/README.md (3 hits) + внутренности internals.md (5 hits) — перехват на testing рядом с ORCH-115.
• LLM-карта/политика/roadmap — llm-call-sites.md (A5), llm-determinization-roadmap.md (rank 2 ✅ реализован; инвариант «ровно один first_slice=yes» цел — yes у rank 1 deployer, no/hybrid_needed=yes у rank 2 tester), llm-usage-policy.md (§5); анти-дрейф-тесты test_llm_call_site_inventory.py/test_llm_determinization_docs.py — зелёные (лично прогнаны).
• Витрина системы docs/overview/ (ORCH-011) — обновлены tech-pipeline.md, tech-agents.md, tech-quality-security.md; tests/test_system_docs.py — зелёный (лично прогнан).
• Промпт .openclaw/agents/tester.md — врезка «детерминированный раннер ведёт стадию; LLM — fallback под выключенным флагом / для репо без контракта»; канон 52d (5 секций, ключ result:) цел.
• CHANGELOG.md — развёрнутая запись [Unreleased].
• ADR заведён (06-adr/ADR-001-deterministic-test-runner.md + сквозной adr/adr-0049-deterministic-test-runner.md).

Обзорные доки / README «Известные ограничения» (ORCH-079): раннер — новая способность, не закрытие документированного ограничения → дополнительных правок README не требуется.

Findings

P0 — Blocker

• Нет.

P1 — Must fix

• Нет.

P2 — Should fix

• Нет.

P3 — Nice-to-have (не блокирует)

• _http_get читает первые 8192 байта тела; smoke ищет serial_gate в ответе /queue. Лично измерено: serial_gate на смещении 1471 байт при полном ответе ~3.6 КБ → запас огромный, сейчас корректно. Латентная хрупкость лишь при экстремальном разрастании ранних блоков; можно опционально поднять лимит чтения для /queue или искать ключ потоково.
• build_test_command добавляет -q, отсутствующий в литеральной команде ADR/CHANGELOG (python -m pytest <target>). Безвредно (exit-код, а не парс вывода, формирует вердикт; согласовано с прежним промптом pytest tests/ -q); фиксирую лишь как расхождение текст↔код.
• Tracked-scratch .task-dev.md перезаписан с ORCH-115 на ORCH-116 (нормальная churn dev-раннера, предсуществующий паттерн, не введён этим PR; вне осей ревью).

Контекст (не finding)

• В истории ветки присутствует 15-staging-log.md со staging_status: FAILED (exit_code: None — staging-сюита не исполнилась, tool-error/инфра, бюджет инфра-ретраев исчерпан → fail-closed). Это исход стадии deploy-staging (downstream от review), относится к staging-раннеру ORCH-115 и является инфра-сбоем, а не дефектом кода ORCH-116 — вне осей код-ревью. Поведение гейта корректно (fail-closed на исчерпании инфра-бюджета); правок в коде задачи не требует. Отмечено как контекст для последующих стадий.

Документация

Обновлена в полном объёме в том же PR (см. ось 4): CLAUDE.md, docs/architecture/README.md, internals.md, llm-call-sites.md, llm-determinization-roadmap.md, llm-usage-policy.md, docs/overview/{tech-pipeline,tech-agents,tech-quality-security}.md, .openclaw/agents/tester.md, CHANGELOG.md, ADR-001 + сквозной adr-0049. Анти-дрейф-тесты документации (test_llm_call_site_inventory.py, test_llm_determinization_docs.py, test_system_docs.py) — зелёные (лично прогнаны). Невыполненных требований к обновлению документации нет.

Вердикт

Нет P0/P1/P2 findings; критический инвариант NFR-1 соблюдён байт-в-байт (stages.py/qg/checks.py не тронуты — проверено лично); документация = golden source обновлена синхронно с кодом; полный регресс зелёный лично проверен (2137 passed). verdict: APPROVED.