From 93cf2732a23e5b0b1a69f93122693786bcf2bcde Mon Sep 17 00:00:00 2001 From: claude-bot Date: Wed, 10 Jun 2026 09:12:04 +0300 Subject: [PATCH] reviewer(ET): auto-commit from reviewer run_id=567 --- docs/work-items/ORCH-100/12-review.md | 117 ++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) create mode 100644 docs/work-items/ORCH-100/12-review.md diff --git a/docs/work-items/ORCH-100/12-review.md b/docs/work-items/ORCH-100/12-review.md new file mode 100644 index 0000000..2b5b9e7 --- /dev/null +++ b/docs/work-items/ORCH-100/12-review.md @@ -0,0 +1,117 @@ +--- +verdict: APPROVED +work_item: ORCH-100 +stage: review +author_agent: reviewer +status: approved +created_at: 2026-06-10 +model_used: claude-opus-4-8 +type: review +work_item_id: ORCH-100 +version: 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.