68 lines
4.7 KiB
Markdown
68 lines
4.7 KiB
Markdown
---
|
||
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.
|