orchestrator/docs/work-items/ORCH-100/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-100	review	reviewer	approved	2026-06-10	claude-opus-4-8	review	ORCH-100	2

Review ORCH-100 — FND/F1b: sidecar-watchdog (re-review)

Summary

Аддитивная реализация sidecar-наблюдателя в отдельном контейнере orchestrator-watchdog (папка watchdog/, тонкий Python-3.12-stdlib-only демон). Это повторное ревью после цикла testing → development: предыдущий прогон тестера дал result: FAIL из-за единственного красного теста tests/test_queue.py::TestRetry::test_finalize_job_requeue_then_fail; развработчик закрыл причину тест-only фиксом (коммит 2040de3 — autouse-фикстура _isolate_runs_dir в tests/conftest.py, без правок src/**).

Проверено по 4 осям. Реализация точно соответствует ТЗ (FR-1…FR-11) и ADR-001 (D1…D9): отдельный контейнер, толерантный парсинг /metrics (D9), debounce orch_down (FR-3, порог orch_down_ticks), read-only docker.sock (_get хардкодит GET — read-only по построению + mount :ro), обобщённая чистая decide (D4, 1:1 семантика disk_watchdog), независимый Telegram-канал (свои токены, ноль импортов src/**), структурный анти-дубль диск-алерта (D6, opt-in потолок), трёхуровневый never-raise (per-source/per-tick/per-send), kill-switch (idle-loop, не exit).

Корневой инвариант соблюдён: PR не трогает src/** ни одной строкой за всю ветку, включая fix-коммит (git diff origin/main...HEAD --stat -- 'src/**' → пусто) ⇒ STAGE_TRANSITIONS / QG_CHECKS / check_* / machine-verdict ключи / схема БД орка — байт-в-байт прежние.

Блокер тестирования снят. Полный регресс pytest tests/ теперь **зелёный (1617 passed)**, профильная сюита tests/watchdog/` — 66/66 PASS. Документация обновлена исчерпывающе.

Вердикт: APPROVED. P0/P1 нет. Ниже — анализ снятого блокера и P2/P3-замечания (не блокируют).

Findings

P0 — Blocker

• (нет)

P1 — Must fix

• (нет)

P2 — Should fix

• ADR-001 D3: docstring-блок структуры host.py упоминает «CPU (loadavg)», которого в реализации нет. ADR D3 (строка с host.py # ... / CPU (loadavg)) перечисляет CPU/inode среди host-метрик, но реестр сигналов D5 сознательно сузил host до host_mem + opt-in host_disk_crit, а host-CPU/«завис» покрыт через agent_hung из /metrics. Сам watchdog/collectors/host.py внутренне консистентен (его docstring явно пишет «CPU ... computed from the /metrics envelope, not here»), inode FR-4 оговорён как «где доступно» — это документированное сужение на стадии архитектуры, не нарушение ТЗ. Замечание косметическое: привести строку D3 в соответствие с D5 (снять «CPU (loadavg)»/inode из блока структуры). Источник: ADR-001 D3/D5, 02-trz.md FR-4.

P3 — Nice-to-have

• CLAUDE.md не обновлён. Паспорт проекта не получил TL;DR-запись о F1b. Прецедент: парная задача F1a (ORCH-099) также отсутствует в CLAUDE.md (grep → 0) — семейство наблюдаемости в паспорте не трекается, а золотой архитектурный док (docs/architecture/README.md) покрывает F1b исчерпывающе. Опционально для единообразия с операционными демонами (disk_watchdog/reaper).
• DockerSockReader.list_containers не вызывается в core.tick (используется только inspect(name) по cfg.containers). Публичный read-метод оставлен для полноты API/тестов (test_docker_readonly.py) — безвреден; при желании пометить как explicit-API.

Анализ снятого блокера (testing FAIL → development fix)

• Причина прежнего FAIL: test_finalize_job_requeue_then_fail (run_id=1/2) читал хвост <settings.runs_dir>/<run_id>.log. Дефолтный runs_dir указывал на прод-каталог /app/data/runs, где на self-hosting-хосте лежат реальные накопленные *.log; реальный 2.log с токеном «429» переключал классификацию permanent → transient (requeue вместо failed). Это ambient prod-pollution окружения, не дефект кода — сам тест байт-в-байт идентичен origin/main, а src/** ORCH-100 не трогает.
• Фикс (коммит 2040de3): autouse-фикстура _isolate_runs_dir редиректит settings.runs_dir на per-test tmp_path ⇒ _run_log_path() резолвится в несуществующий файл ⇒ classify_log_file() возвращает документированный дефолт permanent ⇒ детерминированный, не зависящий от окружения результат для всей сюиты. Зеркалит существующие autouse-фикстуры _no_telegram/_disable_merge_verify/_reset_webhook_secrets.
• Это НЕ «подгонка теста под код»: тело теста не изменено; добавлена только изоляция окружения (test-infra). Фикс улучшает гигиену всей сюиты и устраняет скрытую env-зависимость. Прежний диагноз тестера («реальное красное, ловящее расхождение requeue→finalize в launcher») оказался ошибочным — корень был в загрязнении прод-логами; артефакт тестера (13-test-report.md) не правлю (чужая стадия), фиксирую факт здесь.
• Верификация (независимо): git diff origin/main...HEAD --stat -- 'src/**' → пусто (включая fix-коммит); изолированный прогон test_finalize_job_requeue_then_fail → 1 passed; полный pytest tests/ → 1617 passed; tests/watchdog/ → 66 passed.
• Багфикс-трек (ORCH-019 BR-4): задача — feat/FND (не Bug) ⇒ требование регресс-теста-фиксатора не применяется. Фикс окружения, тем не менее, детерминирует поведение всей сюиты.

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

Обновлена исчерпывающе — golden source синхронизирован с кодом:

• ✅ docs/architecture/README.md — новая компонентная строка (Sidecar-watchdog F1b) + полная секция дизайна F1b + перекрёстная ссылка из секции F1a.
• ✅ CHANGELOG.md — детальная запись F1b (стек D1 / топология D2 / decide D4 / реестр сигналов D5 / анти-дубль D6 / транспорт D7 / never-raise D8) + отдельная строка fix-коммита 2040de3 (_isolate_runs_dir).
• ✅ docs/work-items/ORCH-100/06-adr/ADR-001-sidecar-watchdog.md + сквозной docs/architecture/adr/adr-0033-sidecar-watchdog.md (оба с корректным frontmatter).
• ✅ docs/work-items/ORCH-100/07-infra-requirements.md — разовое инфра-предусловие (сервис в compose, bot/chat watchdog, .env.watchdog, первый запуск).
• ✅ .env.example — канон всех WATCHDOG_* ключей, без реальных секретов (TG_BOT_TOKEN/ TG_CHAT_ID пустые).
• ⚠️ CLAUDE.md — не обновлён (P3, прецедент F1a — допустимо).
• ✅ README «Известные ограничения» (ось ORCH-079): F1b — новая способность (внешний наблюдатель); ни один из 3 открытых пунктов витрины (Telegram-48h / intra-repo task-deps / пакетный автоном Этап 1) не закрывается этим PR ⇒ обновления обзорной витрины не требуется.

src/** НЕ изменён ⇒ P0 «src изменён, документация не обновлена» не активируется; документация при этом обновлена сверх минимума.

Проверки инвариантов (явно)

• git diff origin/main...HEAD --stat -- 'src/**' → пусто за всю ветку, включая fix-коммит (STAGE_TRANSITIONS / QG_CHECKS / check_* / схема БД — байт-в-байт).
• docker.sock смонтирован :ro (compose) И код GET-only по построению (_get хардкодит GET, ни одного мутирующего метода/POST/start/stop/restart/exec) — двойная гарантия read-only (AC-6).
• Нет импорта src/** из watchdog/** (grep → пусто; C-1 / BR-8) — независимый Telegram-транспорт со своими токенами; падение орка не утянет алерт-канал.
• never-raise: per-source (коллекторы _collect_*), per-tick (__main__.run + core._dispatch), per-send (notify/_send) — все три уровня присутствуют (TC-06).
• kill-switch WATCHDOG_ENABLED=false → idle-loop (НЕ exit) — restart-policy не крутит петлю (TC-07).
• mem_limit: 128m + mem_reservation: 32m; stdlib-only (нет requirements/pip-дерева) — тонкость C-3 соблюдена; compose-сервис изолирован (деплой watchdog НЕ пересобирает/рестартит orchestrator).
• Анти-дубль диска (D6/AC-5): host_disk_crit opt-in (disk_crit_enabled=False по умолчанию) на более высоком потолке (97%) — структурно один владелец 85%-события (disk_watchdog/ORCH-063).

Escalation

• Нет открытых эскалаций. Прежняя эскалация ревью v1 / тест-репорта (pre-existing красный тест) — закрыта fix-коммитом 2040de3 (test-only изоляция окружения, src/** не тронут). Полный регресс pytest tests/ зелёный (1617 passed) ⇒ downstream merge-gate re-test (ORCH-043) по этой причине более не упрётся. Отдельная баг-задача на test_finalize_job_requeue_then_fail не требуется: корнем было загрязнение прод-логами, а не дефект src/**.