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	1

Review ORCH-100 — FND/F1b: sidecar-watchdog

Summary

Аддитивная реализация sidecar-наблюдателя в отдельном контейнере orchestrator-watchdog (папка watchdog/, тонкий Python-3.12-stdlib-only демон). Проверено по 4 осям. Реализация точно соответствует ТЗ (FR-1…FR-11) и ADR-001 (D1…D9): отдельный контейнер, толерантный парсинг /metrics, debounce orch_down, read-only docker.sock (GET-only по построению), обобщённая чистая decide, независимый Telegram-канал, структурный анти-дубль диск-алерта (D6), трёхуровневый never-raise, kill-switch.

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

Вердикт: APPROVED. P0/P1, относящихся к этому PR, нет. Ниже — P2-замечания (не блокируют) и одна эскалация по pre-existing красному тесту вне области F1b.

Findings

P0 — Blocker

• (нет)

P1 — Must fix

• (нет)

P2 — Should fix

• Заявление «полный регресс tests/ -q зелёный» неточно (CHANGELOG / AC-7). Полный прогон pytest tests/ даёт 1 падение: tests/test_queue.py::TestRetry::test_finalize_job_requeue_then_fail. Важно: этот тест байт-в-байт идентичен origin/main (git diff origin/main...HEAD -- tests/test_queue.py → пусто), а PR не меняет src/** — падение pre-existing на main, НЕ вызвано ORCH-100 и структурно неустранимо в рамках F1b (ТЗ §2 прямо запрещает правки src/**). Поэтому это не основание для REQUEST_CHANGES этого PR. Действие: скорректировать формулировку «полный tests/ зелёный» → «tests/watchdog/ зелёные (66); один pre-existing красный тест на main вне области F1b» и завести отдельную баг-задачу (см. Escalation). Источник: AC-7 (03-acceptance-criteria.md), CHANGELOG-запись ORCH-100.
• FR-4: host inode/CPU не реализованы как сигналы. ТЗ FR-4 упоминает «диск (% и, где доступно, inode), память, CPU»; ADR-001 D5 (реестр сигналов) сузил host-метрики до host_mem + opt-in host_disk_crit, и этот реестр реализован полностью. Host-CPU (loadavg) и inode в сигналы не превращены — watchdog/collectors/host.py их не собирает (host-CPU/«завис» покрыт через agent_hung из /metrics). Это документированное сужение на стадии архитектуры (FR-4 формулирует inode как «где доступно»), не нарушение. Замечание: docstring ADR D3 для host.py упоминает «CPU (loadavg)», которого в реализации нет — расхождение комментария и кода; либо реализовать host-CPU/inode-сигнал, либо снять упоминание из docstring D3. Источник: 02-trz.md FR-4, ADR-001 D3/D5.

P3 — Nice-to-have

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

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

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

• ✅ docs/architecture/README.md — новая компонентная строка (Sidecar-watchdog F1b) + полная секция «## Sidecar-watchdog F1b (ORCH-100 — design)» + перекрёстная ссылка в секции F1a.
• ✅ CHANGELOG.md — детальная запись F1b (стек D1 / топология D2 / decide D4 / реестр сигналов D5 / анти-дубль D6 / транспорт D7 / never-raise D8).
• ✅ 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_* ключей, без реальных секретов (токены пустые).
• ⚠️ CLAUDE.md — не обновлён (P3, см. выше; прецедент F1a — допустимо).
• ✅ README «Известные ограничения» (ось ORCH-079): F1b — новая способность (внешний наблюдатель), ни один из 3 открытых пунктов витрины не закрывается этим PR ⇒ обновления обзорной витрины не требуется (ось удовлетворена).

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

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

• git diff origin/main...HEAD --stat -- 'src/**' → пусто (src нетронут; STAGE_TRANSITIONS / QG_CHECKS / check_* / схема БД — байт-в-байт).
• docker.sock смонтирован :ro (compose) И код GET-only по построению (_get хардкодит GET, нет ни одного мутирующего метода) — двойная гарантия read-only (AC-6).
• Нет импорта src/** из watchdog/** (C-1 / BR-8) — независимый Telegram-транспорт со своими токенами; падение орка не утянет алерт-канал.
• never-raise: per-source (коллекторы), per-tick (__main__ + core._dispatch), per-send (notify) — все три уровня присутствуют.
• kill-switch WATCHDOG_ENABLED=false → idle-loop (НЕ exit) — restart-policy не крутит петлю.
• mem_limit: 128m + mem_reservation: 32m; stdlib-only (нет requirements/pip-дерева) — тонкость C-3 соблюдена.
• Багфикс-трек (ORCH-019 BR-4): задача — feat/FND, не Bug ⇒ требование регресс-теста-фиксатора не применяется.

Escalation

• Pre-existing красный тест на main: tests/test_queue.py::TestRetry::test_finalize_job_requeue_then_fail падает стабильно (3/3 изолированных прогона) и идентичен origin/main. Он не вызван ORCH-100 и неустраним в области F1b (правки src/** запрещены ТЗ §2). Однако он завалит downstream merge-gate re-test (ORCH-043 гоняет pytest tests/ на догнанной ветке) и нарушает AC-7 «полный tests/ зелёный» на уровне репозитория. Рекомендация: Owner/планировщику завести отдельную баг-задачу (метка Bug, маршрут ORCH-019) на починку test_finalize_job_requeue_then_fail в src/**; без неё прод-выкат ORCH-100 упрётся в merge-gate по причине, не относящейся к F1b. Это эскалация, а не finding против данного PR.