From 068f4438f964df47e1b43d608ea57b80cdcdd08c Mon Sep 17 00:00:00 2001 From: claude-bot Date: Tue, 16 Jun 2026 07:25:33 +0300 Subject: [PATCH] reviewer(ET): auto-commit from reviewer run_id=743 --- docs/work-items/ORCH-116/12-review.md | 138 +++++++++++++++----------- 1 file changed, 80 insertions(+), 58 deletions(-) diff --git a/docs/work-items/ORCH-116/12-review.md b/docs/work-items/ORCH-116/12-review.md index 120ec1f..bd80613 100644 --- a/docs/work-items/ORCH-116/12-review.md +++ b/docs/work-items/ORCH-116/12-review.md @@ -8,93 +8,105 @@ created_at: 2026-06-16 model_used: claude-opus-4-8 type: review work_item_id: ORCH-116 -version: 1 +version: 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 («замена *продюсера* артефакта, не гейта») соблюдён -байт-в-байт, документация (включая витрину `docs/overview/` и LLM-карту/roadmap/политику) обновлена -в том же PR. Полный регресс зелёный (`2137 passed`), новый `tests/test_orch116_test_runner.py` -зелёный (32 теста), LLM-анти-дрейф зелёный. **P0/P1 findings отсутствуют → `APPROVED`.** +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: - **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). + ведёт `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). -- **AC-6** (анти-дрейф) — `git diff` по `src/` затрагивает только `launcher.py`/`config.py`/`main.py`/ - `test_runner.py`; `src/stages.py` (STAGE_TRANSITIONS) и `src/qg/checks.py` (`check_tests_passed`/ - `_parse_tests_verdict`) **не тронуты**; новых таблиц нет (TC-09). + `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-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-карты/политики/витрины, полный регресс — все зелёные. + LLM-карты/политики/витрины, полный регресс — все зелёные (AC-15 проверен личным прогоном). ### 2. Соответствие ADR — PASS Реализация дословно следует ADR-001 (D1–D12) и сквозному adr-0049. Ключевые места сверены: -- **D6.1 (анти-коллизия 52c-`status:` ↔ `_parse_tests_verdict`)** — корректно: `PASS → status: success` - (`SUCCESS` не позитивный и не негативный токен, позитив `PASS` берётся из `result:`), `FAIL → status: - failed` (`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 (`assert agent == "tester"`). Счётчик restart-safe по маркеру в +- **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 - (`advance_stage`/transition-lease — берётся **внутри** `advance_stage`, раннер не трогает, граница O1), - ORCH-110 (`proc_group` переиспользуется как есть) — ни один зафиксированный инвариант не сломан. + (transition-lease берётся **внутри** `advance_stage` — раннер не трогает, граница O1), ORCH-110 + (`proc_group` переиспользуется как есть) — ни один зафиксированный инвариант не сломан. - Глобальные ADR (INV-4 «мерж только через PR», запрет рестарта прод-контейнера) — соблюдены: лог пушится только в фичеветку, никакой работы с `main`. ### 3. Качество кода — PASS - Docstrings на всех публичных функциях; контракт never-raise выдержан сквозно (каждый внешний вызов - обёрнут, наблюдательные счётчики через `_bump` не ломают решение). -- Тесты содержательные (32 TC, не тривиальные): покрыты kill-switch/scope/contract, маппинг по классам - входа, рендер+парсер, перехват, two-level outcome, never-raise, timeout, smoke read-only, - анти-дрейф, наблюдаемость. -- Сигнатуры зависимостей сверены: `db.enqueue_job(..., available_at_delay_s=)`, `db.mark_job`, - `self_deploy.map_exit_code_to_status`, `config.post_deploy_base_url` (дефолт `http://localhost:8500`) - — все существуют, AttributeError-риск smoke отсутствует. + обёрнут; наблюдательные счётчики через `_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 неприменимо; тем не менее покрытие исчерпывающее. +- Это **не** багфикс-трек (стадия `architecture` отработана, full-cycle) → требование + регресс-фиксатора ORCH-019/BR-4 неприменимо; тем не менее покрытие исчерпывающее. ### 4. Документация — PASS (обязательная проверка) -`src/` изменён → документация обновлена **в том же PR**, проверено явно: +`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`» цел; анти-дрейф-тесты зелёные. +- **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». + `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`). @@ -114,22 +126,32 @@ PR заменяет LLM-агента `tester` на стадии `testing` (self- ### P3 — Nice-to-have (не блокирует) - [ ] `_http_get` читает только первые 8192 байта тела (`resp.read(8192)`), а smoke проверяет наличие - `serial_gate` в ответе `/queue`. Эмпирически `serial_gate` лежит на офсете ~1481 байт (запас ~6.7 КБ - до границы) — **сейчас корректно**, но при будущем разрастании блоков `/queue` *перед* `serial_gate` - это латентная хрупкость (ложный smoke-FAIL здорового прогона). Опционально: поднять лимит чтения для - `/queue` или искать ключ потоково. Не блокирует — текущий запас комфортный. + `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 `). Безвредно и согласовано с прежним промптом 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`) — зелёные. Требований к -обновлению документации, оставшихся невыполненными, **нет**. +`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 соблюдён байт-в-байт; документация = golden source -обновлена синхронно с кодом; полный регресс зелёный. **`verdict: APPROVED`.** +Нет P0/P1 findings; критический инвариант NFR-1 соблюдён байт-в-байт (`stages.py`/`qg/checks.py` не +тронуты); документация = golden source обновлена синхронно с кодом; полный регресс зелёный лично +проверен (2137 passed). **`verdict: APPROVED`.**