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	2

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, качество кода, документация) с самостоятельным прогоном тестов. Все 15 критериев приёмки выполнены; критический инвариант NFR-1 («замена продюсера артефакта, не гейта») соблюдён байт-в-байт (только 4 файла в src/, stages.py/qg/checks.py не тронуты); документация = golden source обновлена в том же PR. Полный регресс проверен лично: pytest tests/ -q → 2137 passed; ORCH-116 + LLM-анти-дрейф + system-docs → 74 passed. P0/P1 findings отсутствуют → APPROVED.

Оси проверки

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

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

• AC-1 (перехват до _spawn) — launcher.launch_job: if job.get("agent")=="tester" and test_runner.should_intercept(job): return self._run_test_runner_job(job); _run_test_runner_job ведёт jobs через mark_job, возвращает None (нет agent_runs). TC-05 воспроизводит без CLI.
• 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); второй маппинг не введён (TC-02).
• AC-4 (эквивалентность маршрутизации) — _advance(finished_agent="tester", current_stage="testing"), без новых рёбер; PASS→advance, FAIL→существующий откат testing→development (TC-07/TC-10).
• AC-5 (two-level outcome) — run_test_gate: suite_ran = returncode is not None and not timed_out; tool-error → bounded DEFER re-queue tester-джоба (TC-10 проверяет agent == "tester"), не код-фейл-откат, не жжёт developer-retry.
• AC-6 (анти-дрейф) — git diff по src/ затрагивает только launcher.py/config.py/main.py/test_runner.py; src/stages.py и src/qg/checks.py байт-в-байт не тронуты (проверено git diff лично); новых таблиц нет (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 не роняет воркер (TC-11).
• AC-10 (self-hosting safety) — build_test_command без литералов рестарта/compose/build/force-push/.env; smoke строго read-only GET (TC-13).
• AC-11 (proc_group/worktree/timeout) — pytest в get_worktree_path через run_in_process_group, _resolve_timeout дефолтит на малформе (TC-12).
• AC-12/AC-13/AC-14/AC-15 — гибрид off-control-path, блок test_runner в /queue, обновление LLM-карты/политики/витрины, полный регресс — все зелёные (AC-15 проверен личным прогоном).

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

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

• D6.1 (анти-коллизия 52c-status: ↔ _parse_tests_verdict) — корректно и проверено против неизменённого парсера (src/qg/checks.py:263-277): result: PASS → status: success (конкатенация "SUCCESS PASS" — нет негативного токена, позитив PASS из result: → True); result: FAIL → status: failed ("FAILED FAIL" — негативный токен → False). В build_test_report жёстко: sub_status = "success" if result == "PASS" else "failed". Прибито TC-04/test_tc04_status_field_never_false_negatives_a_pass. Самая тонкая мина задачи — снята верно.
• D5 DEFER — зеркало staging_runner._handle_tool_error, но re-queue tester-джоба (не deployer) — проверено TC-10. Счётчик restart-safe по маркеру test-runner infra-retry в task_content, без правки схемы БД.
• Трассировка маркеров (TRACEABILITY): правки рядом с ORCH-115 (launcher), ORCH-114 (transition-lease берётся внутри advance_stage — раннер не трогает, граница O1), ORCH-110 (proc_group переиспользуется как есть) — ни один зафиксированный инвариант не сломан.
• Глобальные ADR (INV-4 «мерж только через PR», запрет рестарта прод-контейнера) — соблюдены: лог пушится только в фичеветку, никакой работы с main.

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

• Docstrings на всех публичных функциях; контракт never-raise выдержан сквозно (каждый внешний вызов обёрнут; наблюдательные счётчики через _bump не ломают решение).
• Тесты содержательные (32 тест-функции, не тривиальные): kill-switch/scope/contract, маппинг по классам входа, рендер+парсер, перехват, two-level outcome, never-raise, timeout, smoke read-only (включая транзиент-ретрай и missing-serial_gate), анти-дрейф, наблюдаемость.
• Сигнатуры зависимостей сверены лично (риск latent false-FAIL устранён): db.enqueue_job(..., 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) — все существуют ровно как используются.
• Security: smoke — только urllib GET к локальному оркестратору; мутирующих запросов нет.
• Это не багфикс-трек (стадия architecture отработана, full-cycle) → требование регресс-фиксатора ORCH-019/BR-4 неприменимо; тем не менее покрытие исчерпывающее.

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

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

• Паспорт CLAUDE.md — новый раздел «Детерминированный test-раннер (ORCH-116)».
• Компонент-карта docs/architecture/README.md — запись Test-runner + блок roadmap «второй срез реализован».
• Внутренности docs/architecture/internals.md — примечание о перехвате на testing.
• LLM-карта/политика/roadmap — llm-call-sites.md (A5), llm-determinization-roadmap.md (rank 2 ✅), llm-usage-policy.md (§5); инвариант «ровно один first_slice=yes» цел (у rank 1 deployer); анти-дрейф-тесты 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».
• CHANGELOG.md — запись [Unreleased].
• ADR заведён (06-adr/ADR-001 + сквозной adr-0049).

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

Findings

P0 — Blocker

• Нет.

P1 — Must fix

• Нет.

P2 — Should fix

• Нет.

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

• _http_get читает только первые 8192 байта тела (resp.read(8192)), а smoke проверяет наличие serial_gate в ответе /queue. serial_gate — 11-й ключ ответа (src/main.py:261), идёт до основной массы аддитивных блоков (coverage/fs_ownership/auto_labels/stop/bug_fast_track/ checkout_hygiene/transition_lease/staging_runner/test_runner/lessons) → запас комфортный, сейчас корректно. Латентная хрупкость при разрастании ранних блоков — опционально поднять лимит чтения для /queue или искать ключ потоково. Не блокирует.
• build_test_command добавляет флаг -q, отсутствующий в литеральной команде ADR (python -m pytest <target>). Безвредно и согласовано с прежним промптом tester (pytest tests/ -q); фиксирую лишь как расхождение текст↔код.

Контекст (не 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 findings; критический инвариант NFR-1 соблюдён байт-в-байт (stages.py/qg/checks.py не тронуты); документация = golden source обновлена синхронно с кодом; полный регресс зелёный лично проверен (2137 passed). verdict: APPROVED.