diff --git a/docs/work-items/ORCH-109/12-review.md b/docs/work-items/ORCH-109/12-review.md new file mode 100644 index 0000000..82d2615 --- /dev/null +++ b/docs/work-items/ORCH-109/12-review.md @@ -0,0 +1,100 @@ +--- +verdict: REQUEST_CHANGES # APPROVED | REQUEST_CHANGES — строго одно из двух, UPPERCASE +work_item: ORCH-109 +stage: review +author_agent: reviewer +status: changes-requested +created_at: 2026-06-14 +model_used: claude-opus-4-8 +type: review +work_item_id: ORCH-109 +version: 1 +--- + +# Review ORCH-109 + +## Summary + +Две аддитивные изолированные правки подсистемы запуска (`launcher`) — launch-стамп модели в +`agent_runs.model` (D1/FR-1) и поднятые per-role wall-clock бюджеты developer/reviewer с синхронным +поднятием reaper (D3/D4/FR-3) — реализованы **корректно и точно по ADR**. Контракты неприкосновенны: +`STAGE_TRANSITIONS`/`QG_CHECKS`/`check_*`/machine-verdict/схема БД — байт-в-байт (AC-9 верифицирован +по диффу: в `src/` изменены только `launcher.py` и `config.py`, ни одной контракт/схема-строки). +Инварианты ORCH-087 (стамп эффорта — объединён в один `UPDATE`, не сломан) и ORCH-065 (reaper +`5400 > 3600+20`) сохранены и закреплены тестами. Покрытие исчерпывающее: новый +`tests/test_orch109_timeout_model.py` (TC-01…TC-12, детерминированный), полный регресс +**1899 passed** (exit 0). + +**Блокер ровно один — документация.** PR изменил поведение тайм-аута в `src/` и корректно обновил +инженерный справочник `docs/architecture/internals.md` (30 мин → per-role), но **пропустил +front-page витрину `README.md`**: раздел «### Watchdog» (стр. 295) по-прежнему утверждает «Каждый +агент имеет timeout 30 минут», что теперь фактически неверно (developer 60м / reviewer 50м / прочие +30м). Обзорная витрина не должна выдавать устаревший факт за текущий (ORCH-079/ORCH-011) → **P1**, +вердикт `REQUEST_CHANGES`. Правка тривиальна (1 строка), остальной PR — эталонного качества. + +## Оси проверки + +1. **Соответствие ТЗ (02-trz / 03-acceptance):** FR-1…FR-6 реализованы; AC-1…AC-10 покрыты тестами + TC-01…TC-12. ✅ (кроме AC-10 в части README — см. P1). +2. **Соответствие ADR (06-adr ADR-001 + сквозной adr-0040):** D1–D6 реализованы дословно — объединённый + `UPDATE agent_runs SET model=?, effort=? WHERE id=?` с `(model or None, effort or None, run_id)`; + лестница `_resolve_timeout` (overrides_json → выделенный ключ роли → глобальный дефолт); выделенные + ключи `agent_timeout_developer_s=3600`/`agent_timeout_reviewer_s=3000`; `reaper_max_running_s` + 3600→5400. Трассировка маркера **ORCH-087** (стамп эффорта): правка добавляет `model` в тот же + `UPDATE`, эффорт-стамп `(effort or None)` сохранён — зафиксированный инвариант не сломан. ✅ +3. **Качество кода:** never-raise сохранён (`try/except` + WARNING вокруг стамп-UPDATE и вокруг резолва + выделенного ключа); непозитивный/нечисловой ключ → откат на дефолт + WARNING; докстринги/комментарии + точные; тесты содержательные (позитивные контроли `test_tc11_clean_exit_advances`, + `test_tc09_unstamped_killed_run_drops_model_suffix` как negative-guard, параметризация + `[0,-5,"abc"]`). ✅ +4. **Документация (приоритетная ось):** обновлены CHANGELOG / CLAUDE.md / docs/architecture/README.md / + internals.md / .env.example / config.py-паспорт / детальный ADR + сквозной adr-0040. **Один пропуск + во front-page витрине `README.md` → P1** (см. ниже). + +## Findings + +### P0 — Blocker +- (нет) + +### P1 — Must fix +- [ ] **`README.md:294–295` (раздел «### Watchdog») — устаревший факт во front-page витрине.** Строка + «Каждый агент имеет timeout 30 минут. При превышении — SIGKILL + запись exit_code=-9.» противоречит + поведению, введённому этим PR (per-role бюджеты: developer 60м / reviewer 50м / прочие 30м дефолт, + `_resolve_timeout`, ORCH-109). PR изменил `src/` (тайм-аут) и обновил инженерный + `docs/architecture/internals.md`, но **обзорную витрину `README.md` не синхронизировал** — читатель + фронт-страницы видит решённое/изменённое как старое. Нарушает правило агентов №2/№6 и ось «обзорные + доки» (ORCH-079) / «витрина системы» (ORCH-011): необновление обзорной витрины при изменении + описанной в ней функциональности → finding ≥ P1. + **Fix:** привести `README.md:295` в соответствие с `internals.md` — указать per-role бюджеты + (developer 60м / reviewer 50м / прочие 30м дефолт, `_resolve_timeout`, ORCH-109) и, при желании, + упомянуть Tier-3 backstop `reaper_max_running_s`=90м. (Каскад SIGTERM→grace→SIGKILL — pre-existing, + не предмет этого PR; достаточно поправить число «30 минут».) + +### P2 — Should fix +- (нет) + +## Документация + +**Обновлено в этом PR (golden source синхронизирован с кодом):** +- `CHANGELOG.md` — детальная запись ORCH-109 (`fix`, D1/D3/D4, FR-4/FR-5 структурно). ✅ +- `CLAUDE.md` — паспорт (блок «Стек», абзац launcher). ✅ +- `docs/architecture/README.md` — бюллет Agent Launcher (ссылка на adr-0040). ✅ +- `docs/architecture/internals.md` — watchdog 30 мин → per-role (стр. 96 и 262). ✅ +- `.env.example` — новый блок agent-timeout (5 ключей) + `ORCH_REAPER_MAX_RUNNING_S` 3600→5400. ✅ +- `src/config.py` — паспорт-комментарий ORCH-7/ORCH-109 + reaper-инвариант. ✅ +- ADR — `docs/work-items/ORCH-109/06-adr/ADR-001-…` (детальный) + `docs/architecture/adr/adr-0040-…` + (сквозной). ✅ + +**Требует обновления в этом же PR (блокер P1):** +- `README.md:295` (раздел «### Watchdog») — единственный активный устаревший факт «timeout 30 минут» + (см. P1). Прочие совпадения «30 мин/1800/3600» в дереве — замороженные историко-номерные артефакты + work-items (ORCH-053/065/016/098) и `docs/history/*` (правке не подлежат, правило №3), либо корректно + актуализированный `internals.md`. + +**Прочее (не findings):** +- Полный регресс `pytest tests/` — **1899 passed** (exit 0). Предупреждение + `PytestUnhandledThreadExceptionWarning: no such table: agent_runs` в `_monitor_agent` (launcher.py:860) + — pre-existing гонка тестового потока с teardown БД в **неизменённом** этим PR коде (дифф трогает + только `_spawn` ≈566 и `_resolve_timeout` ≈670); не блокер, не регресс ORCH-109. +- AC-9 верифицирован: в `src/` изменены только `launcher.py` и `config.py`; ни одной строки + `STAGE_TRANSITIONS`/`QG_CHECKS`/`check_*`/machine-verdict/`CREATE TABLE`/`ALTER TABLE` в диффе нет.