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

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

@@ -8,166 +8,132 @@ created_at: 2026-06-16
 model_used: claude-opus-4-8
 type: review
 work_item_id: ORCH-116
-version: 3
+version: 4
 ---
 
 # Review ORCH-116 — детерминированный test-раннер вместо LLM-тестера
 
+> Машинный вердикт читается ТОЛЬКО из `verdict:` во frontmatter.
+
 ## 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-0050). Полный регресс
-**проверен лично**: `pytest tests/ -q` → **2137 passed**; ORCH-116 + LLM-анти-дрейф + system-docs →
-**74 passed**. **P0/P1/P2 findings отсутствуют → `APPROVED`.**
+Реализация **полностью соответствует ТЗ (02-trz.md), критериям приёмки (03-acceptance-criteria.md)
+и ADR-001 / adr-0050**. ORCH-116 заменяет LLM-агента `tester` на стадии `testing` детерминированным
+leaf `src/test_runner.py`, перехватываемым в `launch_job` до `_spawn` — точное зеркало эталона
+`src/staging_runner.py` (ORCH-115). Критический инвариант NFR-1 соблюдён байт-в-байт: гейт
+`check_tests_passed` / `_parse_tests_verdict`, `STAGE_TRANSITIONS`, `QG_CHECKS`, machine-key `result:`
+и схема БД **не тронуты** (это замена *продюсера* артефакта, не гейта). Тонкая мина D6.1 (52c-`status:`
+↔ парсер с negative-token-priority) обработана корректно и прибита тестом через неизменённый парсер.
+Документация обновлена в полном объёме (CLAUDE.md / CHANGELOG / README / internals / витрина
+`docs/overview/` / LLM-карта/roadmap/policy / tester.md / .env.example). Все 4 оси — зелёные. Findings
+уровня P0/P1, относящихся к ORCH-116, нет.
 
 ## Оси проверки
 
-### 1. Соответствие ТЗ / Acceptance Criteria — PASS
+### 1. Соответствие ТЗ / AC — PASS
+- **FR-1 / AC-1** перехват до `_spawn`: `launcher.launch_job` врезка `if job.get("agent")=="tester"
+  and test_runner.should_intercept(job): return self._run_test_runner_job(job)` рядом с
+  D1/D2/ORCH-115; `_run_test_runner_job` — зеркало `_run_staging_runner_job`, ведёт `jobs` через
+  `mark_job`, возвращает `None` (нет `agent_runs`). ✔ (TC-05).
+- **FR-2/3 / AC-3/AC-11** pytest в worktree через `proc_group` (tree-kill, `_resolve_timeout`
+  fail-safe), маппинг `map_exit_code_to_result` поверх единого `self_deploy.map_exit_code_to_status`
+  (без второго маппинга). ✔ (TC-02, TC-12).
+- **FR-4 / AC-2** `write_test_report` — `result:` UPPERCASE + 52c-схема, `author_agent: test-runner` /
+  `model_used: n/a`, push только в фичеветку (без merge в `main`). ✔ (TC-03, TC-04).
+- **FR-5 / AC-4** `_advance` вызывает `advance_stage(current_stage="testing", finished_agent="tester")`
+  — FAIL-ветвь `stage_engine.py:849` матчит по `agent=="tester"`. ✔ (TC-07).
+- **FR-6 / AC-5** two-level outcome: `suite_ran=(returncode is not None) and (not timed_out)`;
+  tool-error → bounded DEFER (re-queue именно **`tester`**-джоба + restart-safe маркер), на исчерпании
+  → fail-closed `FAIL` + advance + INFRA-alert. ✔ (TC-10).
+- **FR-7 / AC-7/AC-8** `applies()` = kill-switch + скоуп + `_has_test_contract` (репо без контракта →
+  LLM-tester даже при ручном добавлении в CSV). ✔ (TC-01, TC-08).
+- **FR-8 / AC-13** блок `test_runner` в `GET /queue` + счётчики + структурный лог, различающий
+  code-fail / tool-error. ✔ (TC-14).
+- **FR-9 / AC-12** гибрид: LLM вне control-path вердикта. ✔.
 
-Верифицировано по коду + тестам + личному прогону каждое 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-0050. Сверено критическое:
-- **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.
+### 2. Соответствие ADR / инварианты — PASS
+- NFR-1 / AC-6: `git diff origin/main...HEAD` НЕ затрагивает `src/stages.py`, `src/qg/`,
+  `src/stage_engine.py`, `src/staging_runner.py`, `src/proc_group.py`, `src/transition_lease.py`,
+  `src/git_worktree.py` — проверено явно (пустой diff). ✔.
+- **D6.1 (ключевая мина)**: `_parse_tests_verdict` (`src/qg/checks.py:263-277`) читает вердикт из трёх
+  полей с приоритетом негативного токена. Раннер выравнивает `status:` по вердикту (`PASS→success`
+  «SUCCESS» — не позитивный и не негативный токен → `True` берётся из `result: PASS`; `FAIL→failed`
+  «FAILED» — негативный → `False`). Проверено вручную по логике парсера и тестом TC-04
+  (`test_tc04_status_field_never_false_negatives_a_pass`) через **неизменённый** парсер. ✔.
+- Граница O1: `staging_runner` (ORCH-115), `transition_lease` (ORCH-114), `checkout_hygiene`
+  (ORCH-112) не модифицированы — соответствует требованию. ✔.
+- Трассировка: новые врезки корректно ссылаются на прецеденты D1/D2/ORCH-115; зафиксированные
+  инварианты конвейера не сломаны. ✔.
 
 ### 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 не применяется; покрытие тем не менее
-  исчерпывающее.
+- never-raise соблюдён во всех публичных функциях (`applies`/`should_intercept`/`run_test_gate`/
+  `snapshot`/`write_test_report`/`run_smoke` — guarded; TC-01/TC-06/TC-11/TC-14 покрывают).
+- Лениво импортируются тяжёлые модули — leaf-чистота сохранена (импорт на уровне модуля только
+  `config`/`logging`/`proc_group`).
+- Тесты содержательные (14 TC + анти-дрейф TC-15), без живого Claude CLI/сети; покрывают happy/FAIL/
+  tool-error/never-raise/kill-switch/scope/smoke/observability.
+- Это feature-трек (не `Bug`), регресс-тест-фиксатор (ORCH-019 BR-4) не требуется; покрытие тем не
+  менее исчерпывающее.
 
 ### 4. Документация — PASS (обязательная проверка)
+`src/` изменён → документация обновлена в том же PR, проверено пофайлово:
+- `CLAUDE.md` — новый раздел «Детерминированный test-раннер (ORCH-116)». ✔
+- `CHANGELOG.md` — детальная запись `[Unreleased]`. ✔
+- `docs/architecture/README.md` — компонент **Test-runner** + блок roadmap «второй срез реализован». ✔
+- `docs/architecture/internals.md` — примечание о перехвате на `testing`. ✔
+- `docs/architecture/llm-call-sites.md` (A5), `llm-determinization-roadmap.md` (rank 2),
+  `llm-usage-policy.md` (§5) — обновлены; **инвариант «ровно один `first_slice=yes`» сохранён**
+  (deployer=yes, tester=no), анти-дрейф `tests/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 цел). ✔
+- `.env.example` — все 7 ключей `ORCH_TEST_RUNNER_*`. ✔
+- ADR + сквозной `adr-0050` присутствуют. ✔
 
-`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-0050-deterministic-test-runner.md`).
-
-Обзорные доки / README «Известные ограничения» (ORCH-079): раннер — **новая способность**, не
-закрытие документированного ограничения → дополнительных правок README не требуется.
+## Тесты (AC-15)
+- `tests/test_orch116_test_runner.py` + LLM анти-дрейф — **45 passed**.
+- Полный регресс `pytest tests/ -q` — **2162 passed, 1 failed** (84s). Единственный фейл —
+  `tests/test_orch123_staging_runner_exec.py::test_r2_held_deploy_staging_not_rolled_back` (см. P2).
 
 ## Findings
 
 ### P0 — Blocker
-- Нет.
+- (нет)
 
 ### P1 — Must fix
-- Нет.
+- (нет)
 
 ### P2 — Should fix
-- Нет.
+- **[Вне ORCH-116, для отдельного follow-up] Pre-existing test-isolation flake в ORCH-123.**
+  В полном прогоне падает `tests/test_orch123_staging_runner_exec.py::test_r2_held_deploy_staging_
+  not_rolled_back`. Доказано, что это **не** дефект ORCH-116: (1) тест зелёный в изоляции и в своём
+  файле; (2) фейл воспроизводится при прогоне полного сьюта **без** `tests/test_orch116_test_runner.py`
+  (`--ignore`) → 1 failed; (3) ORCH-116 не трогает `src/staging_runner.py` (граница O1, корректно).
+  Это загрязнение глобального состояния от соседнего теста в ORCH-123-коде (недавно влит в `main`).
+  AC-15 в части «ORCH-116 не краснит ранее зелёные тесты» удовлетворён. Рекомендация: завести
+  отдельный bug на изоляцию теста ORCH-123 — он вне области ORCH-116 и не должен блокировать этот PR.
 
-### 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 на исчерпании инфра-бюджета); правок в коде задачи не требует. Отмечено как
-  контекст для последующих стадий.
+### P3 — Nice to have
+- `run_smoke` (`src/test_runner.py:291`) дублирует литерал `"http://localhost:8500"`, который уже
+  является дефолтом `settings.post_deploy_base_url`. Это безвредный defensive double-default
+  (`localhost`/`8500` НЕ в FORBIDDEN-списке `test_no_host_hardcodes` — нарушения нет); можно опереться
+  только на дефолт config. Косметика.
+- Smoke-провал из-за транзиентного блипа прод-8500 после bounded-ретраев даёт `FAIL` → откат +
+  расход developer-retry (формально инфра, трактуемая как код-фейл — родственно классу ORCH-110).
+  Архитектор **осознанно** взвесил это в ADR (D3 + Consequences «−»): bounded smoke-ретрай +
+  config-gate `test_runner_smoke_enabled` + developer-retry-cap; поведение эквивалентно прежнему
+  LLM-tester (`curl /health`). Принято как задокументированный компромисс; не блокирует.
 
 ## Документация
-Обновлена в полном объёме в том же 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-0050. Анти-дрейф-тесты документации
-(`test_llm_call_site_inventory.py`, `test_llm_determinization_docs.py`, `test_system_docs.py`) —
-зелёные (лично прогнаны). Невыполненных требований к обновлению документации **нет**.
+Обновлена в полном объёме в том же PR (см. ось 4). Изменение `src/` сопровождено обновлением
+паспорта, чейнджлога, архитектурной карты, internals, витрины `docs/overview/`, LLM-карты/roadmap/
+политики, промпта `tester.md`, `.env.example` и ADR (локальный + сквозной). Машинно-проверяемые факты
+витрины и LLM-карты держатся зелёными анти-дрейф-тестами. Требование «документация = golden source»
+выполнено → блокирующих документационных findings нет.
 
 ## Вердикт
-Нет P0/P1/P2 findings; критический инвариант NFR-1 соблюдён байт-в-байт (`stages.py`/`qg/checks.py`
-не тронуты — проверено лично); документация = golden source обновлена синхронно с кодом; полный
-регресс зелёный лично проверен (2137 passed). **`verdict: APPROVED`.**
+Нет P0/P1, относящихся к ORCH-116. Все четыре оси (ТЗ, ADR/инварианты, качество кода, документация)
+пройдены; критические инварианты сохранены байт-в-байт; покрытие исчерпывающее. **APPROVED.**
+Единственный красный тест полного сьюта — pre-existing flake ORCH-123 (P2), доказанно независимый от
+ORCH-116 и вне его области правки.