From 3022a5264937ea6ae00727e7a541e5ff89b603e2 Mon Sep 17 00:00:00 2001 From: claude-bot Date: Mon, 15 Jun 2026 02:02:38 +0300 Subject: [PATCH] reviewer(ET): auto-commit from reviewer run_id=677 --- docs/work-items/ORCH-111/12-review.md | 121 ++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 docs/work-items/ORCH-111/12-review.md diff --git a/docs/work-items/ORCH-111/12-review.md b/docs/work-items/ORCH-111/12-review.md new file mode 100644 index 0000000..52252e2 --- /dev/null +++ b/docs/work-items/ORCH-111/12-review.md @@ -0,0 +1,121 @@ +--- +verdict: APPROVED +work_item: ORCH-111 +stage: review +author_agent: reviewer +status: approved +created_at: 2026-06-15 +model_used: claude-opus-4-8 +type: review +work_item_id: ORCH-111 +version: 1 +--- + +# Review ORCH-111 + +## Summary + +PR добавляет в sidecar-watchdog новый opt-in сигнал `proc_blocking` — алерт на долго живущий +осиротевший тест-процесс (pytest), репарентированный на хост и грузящий CPU (инцидент: +`test_install_lite_script.py` жил >2 суток без единого алерта, валил merge-gate re-test на ORCH-109). + +Изменение **строго внутри наблюдателя** (`watchdog/**` + сервис watchdog в `docker-compose.yml`). +Проверено по всем 4 осям — **замечаний уровня P0/P1 нет**. Реализация 1:1 соответствует ТЗ и ADR-001 +(D1–D7), полный `pytest tests/` зелёный (**1933 passed**), watchdog+key-sync тесты зелёные +(125 passed), `src/**` байт-в-байт не тронут, конвейерные контракты сохранены, документация +обновлена в том же PR. **Вердикт: APPROVED.** + +### Сверка по осям + +**1. Соответствие ТЗ / Acceptance Criteria — PASS.** +- FR-1 (чистый builder `proc_signals`, per-entity key `("proc_blocking", pid)`, active ⇔ + `age_s > cfg.proc_age_s`, действенный RU-`detail`: PID + возраст + усечённый cmdline + CPU) — ✓. +- FR-2 (коллектор `watchdog/collectors/proc.py`, stdlib-only, read-only, never-raise → `[]`, чистый + разбор отделён от I/O) — ✓. +- FR-3 (4 ключа `WATCHDOG_PROC_*`, never-raise парсеры, дефолт-off, гейт на `proc_enabled`) — ✓. +- FR-4 (только наблюдение — нет `os.kill`/сигналов/`subprocess`/чтения `/proc//environ`), + подтверждено AST-тестом `test_tc04_collector_source_is_read_only` — ✓. +- FR-5 (диспетч через существующую `decision.decide()`/`AlertState`; RECOVERY синтезируется без новой + анти-спам-логики) — ✓. +- FR-6 (без дубля с `agent_hung` — по построению: cmdline-скоуп + порог возраста) — ✓. +- AC-1…AC-10 покрыты тестами (TC-01 regress-якорь, TC-02 порог/граница, TC-03 конфиг/kill-switch, + TC-04 never-raise/read-only, TC-05 anti-spam→realert→recovery, TC-06 partition vs agent_hung, + TC-07 tick→dispatch + flag-off + never-raise, TC-12 compose `pid: host`). +- §4 (API): выбран watchdog-side путь, `GET /metrics`/`schema_version` не тронуты — ✓. +- §7 (порог): дефолт `WATCHDOG_PROC_AGE_MIN=60` (=3600s) > `max(merge_retest_timeout_s=600, + coverage_run_timeout_s=900)=900s` (4× запас) — кросс-инвариант D2 соблюдён, анти-false-positive — ✓. + +**2. Соответствие ADR — PASS.** +- Реализация соответствует ADR-001 D1 (watchdog-side `/proc` под `pid: host`, НЕ orch-side `/metrics`), + D2 (анти-FP/анти-дубль по построению), D3 (коллектор), D4 (builder + синтез RECOVERY), D5 (конфиг + дефолт-off), D6 (`pid: host` только на `orchestrator-watchdog`), D7 (инварианты конвейера). +- Сквозной ADR `docs/architecture/adr/adr-0041-...` создан (архитектором, commit `1d87ae5`). +- **Трассировка (TRACEABILITY.md):** правки `watchdog/{core,signals,config}.py` и сервиса watchdog в + compose — **аддитивные врезки**, маркированные инварианты предшественников (ORCH-100 sidecar, + read-only-маунты) не сломаны. Существующий тест `test_host_paths_mounted_read_only` остаётся зелёным + (`pid` — не volume), плюс добавлен позитивный `test_watchdog_shares_host_pid_namespace` (и проверка, + что прод `orchestrator` НЕ получает `pid`). +- Глобальные инварианты (AC-9): `STAGE_TRANSITIONS` / `QG_CHECKS` / `check_*` / machine-verdict / + схема БД / `/metrics`+`schema_version` — **байт-в-байт не тронуты** (подтверждено: `git diff` по + `src/` пуст). + +**3. Качество кода — PASS.** +- Docstrings на всех публичных функциях (коллектор/builder/synth) — содержательные, со ссылками на + D-решения ADR. +- Тесты содержательные, не тривиальные: red→green regress-якоря (TC-01 builder, TC-07 tick→dispatch + «алерт даже когда ни одна стадия не stuck»), граничные (strict `>`), патологический `comm` со + скобками/пробелами, гонка «процесс умер mid-scan», AST-скан read-only-контракта, полная + последовательность alert→none→realert→recovery→no-repeat. +- **Регресс-тест дефекта (ORCH-019 BR-4 / усиление):** задача эскалирована `escalate: full-cycle` + (прошла `architecture`), но как баг несёт явные тест-фиксаторы дефекта — требование выполнено. +- Безопасность: read-only, never-raise (per-pid + top-level), `/proc//environ` не читается + (секреты), cmdline усечена в алерте (анти-утечка аргументов, R-2). Привилегия `pid: host` — + read-only, только у наблюдателя, обоснована в `07-infra-requirements.md`. + +**4. Документация — PASS.** +- `src/**` НЕ изменён → жёсткое правило «src изменён, доки нет → P0» не активируется; доки наблюдателя + обновлены в том же PR независимо: + `CHANGELOG.md` (детальная запись), `docs/architecture/README.md` (раздел `proc_blocking`), + `docs/deployment/LITE_SETUP.md` (opt-in блок), ADR work-item `06-adr/ADR-001` + сквозной `adr-0041`, + `.env.watchdog.example` ↔ блок `WATCHDOG_*` `.env.example` (key-sync тест `test_lite_setup_doc.py` + зелёный). +- Стадийное владение артефактами соблюдено (rule 3/4): TRZ/AC/test-plan — analyst (`a0218a2`), + ADR/infra/risks/architecture-README — architect (`1d87ae5`), код/CHANGELOG/env/compose/LITE_SETUP — + developer (`6c83191`). Ретроактивных правок ТЗ/ADR нет. + +## Findings + +### P0 — Blocker +- Нет. + +### P1 — Must fix +- Нет. + +### P2 — Should fix +- Нет. + +### P3 — Nice to have (не блокируют) +- [ ] Опечатка в комментарии `watchdog/collectors/proc.py` (top-level `except`): «one signal tih» → + «one signal **тих**» (косметика). +- [ ] Витрина `docs/overview/` (ORCH-011): описывает sidecar-watchdog абстрактно («опрашивает + `/metrics`, шлёт алерты»), каталог сигналов не перечисляет. Новый opt-in (дефолт-off) сигнал + абстрактную формулировку не противоречит, машинно-проверяемые факты не меняет + (`tests/test_system_docs.py` зелёный) → обновление витрины **не требуется**. Опционально можно + упомянуть новую способность наблюдателя (видимость хост-процессов) — не блокирует. +- [ ] Индекс `docs/architecture/adr/README.md` не содержит `adr-0041` — но это **предсуществующий + долг** (отсутствуют также `adr-0038`/`0039`/`0040` из 3 ранее влитых PR), не свойство этого PR и не + per-PR-контракт. Не вменяется ORCH-111. + +## Документация + +Документация обновлена корректно и в полном объёме **в том же PR**: +- `CHANGELOG.md` — запись `[Unreleased]` с разбивкой по D-решениям. ✓ +- `docs/architecture/README.md` — раздел `proc_blocking` в описании sidecar-watchdog. ✓ +- `docs/deployment/LITE_SETUP.md` — opt-in блок с проверкой (NFR-5). ✓ +- `docs/work-items/ORCH-111/06-adr/ADR-001-...` + сквозной `docs/architecture/adr/adr-0041-...`. ✓ +- `.env.watchdog.example` ↔ `.env.example` (блок `WATCHDOG_*`) — синхронизированы, key-sync тест + зелёный. ✓ + +Изменения функционала наблюдателя задокументированы, конвейерные контракты не тронуты, выкат +пересобирает только `orchestrator-watchdog` (прод `orchestrator` не рестартится, NFR-3). +**Документация = golden source: требование выполнено.**