reviewer(ET): auto-commit from reviewer run_id=160
This commit is contained in:
67
docs/work-items/ORCH-044/12-review.md
Normal file
67
docs/work-items/ORCH-044/12-review.md
Normal file
@@ -0,0 +1,67 @@
|
||||
---
|
||||
type: review
|
||||
work_item_id: ORCH-044
|
||||
verdict: APPROVED
|
||||
version: 1
|
||||
---
|
||||
|
||||
# Review ORCH-044
|
||||
|
||||
## Summary
|
||||
PR закрывает две системные дыры слоя запуска агента (инцидент ORCH-17): **P1** — token-free
|
||||
auth-гейт в preflight, **P3** — «пустой лог / нет result-JSON ⇒ провал». **P2 (`--effort`)
|
||||
корректно исключён** из scope владельцем и вынесен в ORCH-50 — код effort (`_spawn`,
|
||||
`resolve_agent_effort`, `agent_effort_*`) не тронут, что соответствует коррекции в 02-trz.md
|
||||
и ADR-001.
|
||||
|
||||
Реализация полностью соответствует ТЗ и ADR-001. Документация обновлена в том же PR
|
||||
(README.md, internals.md, INFRA.md, CHANGELOG.md, ADR заведён). Тесты зелёные
|
||||
(`pytest tests/ -q` → 504 passed; новые `test_preflight_auth.py` + `test_empty_log_failure.py`
|
||||
покрывают AC-1…AC-6, AC-10…AC-14). Verdict: **APPROVED**.
|
||||
|
||||
## Соответствие ТЗ / AC
|
||||
- **P1 (TR-1.1…1.7):** `preflight._check_auth()` — чтение `<AGENT_HOME>/.credentials.json`,
|
||||
валидация `claudeAiOauth.accessToken` + `expiresAt` (epoch ms, skew), never-raise fail-safe.
|
||||
Путь резолвится от `AgentLauncher.AGENT_HOME` (новый `_agent_home()`, зеркально `_claude_bin()`),
|
||||
а не от HOME процесса орка (TR-1.3 ✓). Встроено в кешируемый `_compute()` (TR-1.4 ✓).
|
||||
Гейтинг клейма не требовал правок `_drain_once` (TR-1.5 ✓ — подтверждено
|
||||
`test_worker_does_not_claim_when_auth_fails`). AC-1/2/3/4/5/6 покрыты тестами.
|
||||
- **P3 (TR-3.1…3.5):** `_validate_result()` (лог непустой + trailing result-JSON по контракту
|
||||
`usage._extract_last_json_object`), `success = exit 0 AND result_ok`. Побочные эффекты успеха
|
||||
(`_post_usage_comments`, `_try_advance_stage`) выполняются только при `success`; при пустом
|
||||
результате — Telegram-алерт + маршрутизация в провал через `_finalize_job(result_ok=False)`.
|
||||
Реальный `exit_code` пишется в `agent_runs` без искажения (отдельный флаг — A4 из ADR).
|
||||
AC-10/11/12/13/14 покрыты тестами (включая `test_never_running_after_empty_result`,
|
||||
permanent/transient-классификацию).
|
||||
- **P1b защитная сетка:** `_handle_auth_marker()` + `is_auth_failure_text()` сбрасывают
|
||||
preflight-кеш при маркере разлогина в логе пути провала; не transient, breaker не крутится.
|
||||
|
||||
## Соответствие ADR
|
||||
Реализация дословно следует ADR-001 (§P1 шаги 1–6, §P3 валидация + finalize, §Конфигурация:
|
||||
`preflight_check_auth`/`claude_credentials_path`/`auth_expiry_skew_seconds`). Альтернативы A4/A5
|
||||
отражены в коде (отдельный `result_ok` вместо подмены exit_code; общий TTL вместо отдельного
|
||||
кеша). Verified: `usage._extract_last_json_object` и `preflight.reset_cache` существуют.
|
||||
|
||||
## Findings
|
||||
|
||||
### P0 — Blocker
|
||||
- нет
|
||||
|
||||
### P1 — Must fix
|
||||
- нет
|
||||
|
||||
### P2 — Should fix
|
||||
- нет (опционально: PydanticDeprecation warning в `config.py:4` — предсуществующий, вне scope ORCH-044).
|
||||
|
||||
## Документация
|
||||
Обновлена корректно и в том же PR (правило агентов №2/№6, AC-15):
|
||||
- `docs/architecture/README.md` — описание Preflight (auth) и Agent Launcher (валидация результата);
|
||||
- `docs/architecture/internals.md` — §4 «Валидация результата», постфактум auth-детекция, таблица resilience, диаграмма `_finalize_job(result_ok)`;
|
||||
- `docs/operations/INFRA.md` — env-карта (3 новые настройки) + раздел «Preflight auth-гейт» с риском R-1;
|
||||
- `CHANGELOG.md` — запись `[Unreleased] / Added`;
|
||||
- ADR `06-adr/ADR-001-preflight-auth-and-empty-result-failure.md` заведён; `10-tech-risks.md` присутствует.
|
||||
|
||||
## Self-hosting (AC-17)
|
||||
Изменения только в слое preflight/launch — не требуют рестарта прод-контейнера в рамках задачи.
|
||||
Выкатка через обязательный staging-гейт (8501) перед прод. Риск ложноположительного auth-fail
|
||||
(R-1) митигирован тумблером `ORCH_PREFLIGHT_CHECK_AUTH` и проверкой на staging.
|
||||
Reference in New Issue
Block a user