Files
orchestrator/docs/work-items/ORCH-109/12-review.md
claude-bot 464bdb2f96
Some checks failed
CI / test (push) Has been cancelled
CI / test (pull_request) Successful in 3m48s
reviewer(ET): auto-commit from reviewer run_id=665
2026-06-14 02:11:15 +03:00

8.2 KiB
Raw Blame History

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
REQUEST_CHANGES ORCH-109 review reviewer changes-requested 2026-06-14 claude-opus-4-8 review ORCH-109 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): D1D6 реализованы дословно — объединённый 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:294295 (раздел «### 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 в диффе нет.