diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e3cbfb..7f71c11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ## [Unreleased] ### Added +- **Надёжность запуска агента: preflight ловит авторизацию + пустой результат = провал** (ORCH-044): закрыты две системные дыры, из-за которых разлогиненный/«быстро умерший» агент тихо вешал общую очередь всех проектов (инцидент ORCH-17). **P1 — preflight ловит auth (token-free, без сети/prompt-ping, BR-1):** после успешного `claude --version` (который отвечает даже когда claude разлогинен — версия локальна) `src/preflight.py` читает `/.claude/.credentials.json` и валидирует OAuth-токен — нет файла / битый JSON / нет `claudeAiOauth.accessToken` ⇒ FAIL; `claudeAiOauth.expiresAt` (epoch ms) `<= now + ORCH_AUTH_EXPIRY_SKEW_SECONDS` ⇒ протух ⇒ FAIL; нет `expiresAt` ⇒ OK (не плодим ложных срабатываний). Путь к credentials резолвится от `AgentLauncher.AGENT_HOME` (`/home/slin`, HOME под которым launcher реально спавнит claude), а не от HOME процесса орка (новый `_agent_home()`, зеркально `_claude_bin()`). Результат кешируется тем же `ORCH_PREFLIGHT_CACHE_TTL`. При `auth=fail` job не клеймится (`_drain_once` уже корректен при `ok=False`), reason виден в `/queue`. Защитная сетка постфактум: `_handle_auth_marker` детектит маркер разлогина в run-логе (`is_auth_failure_text`) и сбрасывает preflight-кеш, чтобы следующий тик переоценил auth (auth-провал НЕ transient, breaker не крутится). Новые настройки: `ORCH_PREFLIGHT_CHECK_AUTH` (тумблер, default true), `ORCH_CLAUDE_CREDENTIALS_PATH` (явный путь), `ORCH_AUTH_EXPIRY_SKEW_SECONDS`. **P3 — пустой лог / нет result-JSON ⇒ провал:** `exit_code==0` больше не считается успехом сам по себе — `_monitor_agent` валидирует результат (`_validate_result`: лог непустой + есть trailing result-JSON по контракту `usage._extract_last_json_object`); `success = exit 0 AND result_ok`. Только при `success` постится «успешный» status-коммент и вызывается `_try_advance_stage`; при `exit 0 & not result_ok` — Telegram-алерт, стадия НЕ двигается, `_finalize_job(result_ok=False)` маршрутизирует job в провал (`empty run log / no result JSON`: по умолчанию permanent → requeue/`failed`+алерт; transient-маркер в логе → transient-путь). Реальный `exit_code` пишется в `agent_runs` без искажения — решение done/fail несёт отдельный флаг `result_ok` (не подменённый код выхода). Итог: `exit 0` всегда завершается терминально/ретраябельно (`done`|`failed`|`queued`) — путь «быстрая смерть с exit 0 → вечный running» закрыт. ⛔ Scope: `--effort` (P2) исключён владельцем и вынесен в ORCH-50 — не трогался. ADR `docs/work-items/ORCH-044/06-adr/ADR-001-preflight-auth-and-empty-result-failure.md`. Тесты: `tests/test_preflight_auth.py`, `tests/test_empty_log_failure.py`. - **Дословный текст findings reviewer/tester встраивается в `task_desc` заворота** (ORCH-046): при откате на `development` строка `task_desc` (попадает в `.task-dev.md` developer-агента) теперь несёт суть претензий, а не только ссылку на файл — устраняет «испорченный телефон», из-за которого агент шёл «читать файл», терял ключевые P0/P1 / причину FAIL и заворачивался снова, выжигая `MAX_DEVELOPER_RETRIES` и токены. Новый defensive-модуль `src/review_parse.py` (контракт «never raise», как `src/frontmatter.py`): `extract_review_findings(path)` — дословные пункты P0/P1 из секции `## Findings` файла `12-review.md`; `extract_test_failures(path)` — релевантный фрагмент тела `13-test-report.md` (приоритет `## Вывод pytest` → FAIL-строки `## Результаты` → `## Итог`). Обе функции усекают результат до `MAX_FINDINGS_CHARS`/`MAX_FAILURES_CHARS` (≈2000) с маркером `…(truncated)`. Две rollback-ветки `src/stage_engine.py` (reviewer REQUEST_CHANGES, tester `check_tests_passed` FAIL) встраивают извлечённый текст и **сохраняют ссылку** на полный файл («Полный контекст»); при пустом/битом артефакте — graceful-фоллбэк на прежнюю ссылку-строку (никаких исключений в `advance_stage`). Tester-ветка дополнительно всегда включает `reason` гейта. Последовательность отката, `_developer_retry_count`, поля `AdvanceResult` и реестр `QG_CHECKS` не менялись. ADR `docs/work-items/ORCH-046/06-adr/ADR-001-embed-findings-in-task-desc.md`. Тесты: `tests/test_review_parse.py`, `tests/test_stage_engine.py::TestRollbackTaskDescEmbedding`. - **Поллинг с ретраем в quality-gate `check_ci_green`** (ORCH-045): гейт CI превращён из single-shot в polling, чтобы устранить race condition — раньше один опрос combined commit-status сразу после пуша developer-а ловил транзиентный `pending` (типично 1-3с, реальный кейс ORCH-017: опрос 17:58:54 → pending, CI дозеленел 17:58:55) и задача застревала насмерть без повторного опроса. Теперь: `success` → пропуск сразу; `failure`/`error` → провал сразу (терминально, ретрай бессмыслен); `pending`/unknown → `time.sleep` и повторный опрос до `ci_poll_max_attempts` раз; истечение попыток → явный `(False, "CI still pending after s")` (тупик больше не молчаливый); 404 → как раньше; транзиентная `httpx.HTTPError` на попытке логируется и ретраится в рамках бюджета. Параметры — новые настройки `ORCH_CI_POLL_MAX_ATTEMPTS` (12) и `ORCH_CI_POLL_INTERVAL_S` (10) в `src/config.py` (~2 мин ожидания pending). Сигнатура `check_ci_green(repo, branch)` и реестр `QG_CHECKS` не менялись; `check_tests_passed` не затронут. ADR `docs/architecture/adr/adr-0004-ci-poll-retry.md`. Тесты: `tests/test_qg.py::TestCheckCIGreen`. - **Прямые ссылки на BRD и Plane-таску в Telegram-уведомлении об апруве** (ORCH-017): пингующее сообщение `notify_approve_requested` теперь встраивает две HTML-``-ссылки — на `docs/work-items//01-brd.md` (Gitea branch-view: `gitea_public_url`→`gitea_url`) и на issue в Plane (`{web_base}/{workspace}/projects/{project_id}/issues/{plane_issue_id}/`). Новая настройка `ORCH_PLANE_WEB_URL` (внешний браузерный web-URL Plane; фолбэк на `plane_api_url`). **Loopback-guard:** если итоговый Plane web-base указывает на localhost/127.0.0.1/0.0.0.0/::1 или пуст — Plane-ссылка опускается (не выпускаем битый localhost-URL). Graceful degradation: каждая ссылка строится независимо и опускается при нехватке данных, сообщение и призыв «Переведите задачу в статус Approved …» сохраняются всегда; ровно одно пингующее сообщение, разделяемая `send_telegram` не тронута. Динамические подписи экранируются `html.escape`, `parse_mode=HTML` сохранён. ADR `docs/work-items/ORCH-017/06-adr/ADR-001-telegram-approve-links.md`. Тесты: `test_notify_approve_links.py`, `test_analysis_approve_flow_links.py`. diff --git a/docs/architecture/README.md b/docs/architecture/README.md index 9fdfe85..2b15ea2 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -9,8 +9,9 @@ - **Stage Engine** (`src/stage_engine.py`) — исполнение переходов, диспетчеризация QG (`_run_qg`), откаты, синхронизация с Plane. - **Review/Test Parsers** (`src/review_parse.py`, ORCH-046) — defensive-извлечение дословного must-fix текста из артефактов для встраивания в `task_desc` заворота: `extract_review_findings` (P0/P1 из `12-review.md`), `extract_test_failures` (фрагмент тела `13-test-report.md`). Контракт «never raise»: любая ошибка → `""`. - **Quality Gates** (`src/qg/checks.py`) — проверки выхода со стадии, реестр `QG_CHECKS`. -- **Agent Launcher** (`src/agents/launcher.py`) — запуск Claude CLI агентов в изолированном git worktree, мониторинг, auto-advance. -- **Queue** (`src/queue_worker.py`, ORCH-1) — персистентная очередь задач (SQLite `jobs`), atomic claim, max_concurrency, ретраи, restart-safe. +- **Agent Launcher** (`src/agents/launcher.py`) — запуск Claude CLI агентов в изолированном git worktree, мониторинг, auto-advance. **Валидация результата (ORCH-044):** `exit_code==0` считается успехом только если run-лог непустой и содержит валидный result-JSON; пустой/невалидный результат ⇒ job `failed`/retry + алерт, без авто-advance и «успешного» коммента. +- **Preflight** (`src/preflight.py`, ORCH-1/ORCH-044) — дешёвый token-free гейт клейма: `os.path.exists(bin)` + `claude --version` + **проверка авторизации** (чтение `/.claude/.credentials.json` и валидности `claudeAiOauth.expiresAt`; постфактум-маркер `Not logged in`). Кешируется на `preflight_cache_ttl`. Подробнее: [ADR work-item ORCH-044](../work-items/ORCH-044/06-adr/ADR-001-preflight-auth-and-empty-result-failure.md). +- **Queue** (`src/queue_worker.py`, ORCH-1) — персистентная очередь задач (SQLite `jobs`), atomic claim, max_concurrency, ретраи, restart-safe. Не клеймит job при `preflight=fail` (в т.ч. auth-fail). - **Project Registry** (`src/projects.py`, ORCH-6) — Plane project id → repo + prefix; фильтрация вебхуков по проекту. - **Plane Sync** (`src/plane_sync.py`) — синхронизация статусов/комментариев в Plane. diff --git a/docs/architecture/internals.md b/docs/architecture/internals.md index 3e01346..b130b51 100644 --- a/docs/architecture/internals.md +++ b/docs/architecture/internals.md @@ -88,7 +88,16 @@ claude.exe --print --system-prompt --allowedTools Read,Write,Edit,Bash 1. Записывает run в DB (agent_runs) 2. Запускает subprocess. **stdout/stderr перенаправляются СРАЗУ в файл `/app/data/runs/{id}.log` на уровне ОС** (Popen `stdout=log_fh`). Никакого PIPE в памяти оркестратора → нет PIPE-deadlock, нет потока-читателя, нет зомби (B-2). 3. Стартует **watchdog thread** (timeout 30 мин → SIGKILL по pid) -4. Стартует **monitor thread**: `proc.wait()` (гарантированный reap → реальный exit_code в БД) → закрывает log_fh → git commit/push → auto-advance +4. Стартует **monitor thread**: `proc.wait()` (гарантированный reap → реальный exit_code в БД) → закрывает log_fh → **валидация результата (ORCH-044)** → git commit/push → auto-advance + +**Валидация результата (ORCH-044, P3).** `exit_code==0` сам по себе НЕ считается успехом: claude может «быстро умереть» (разлогинен / флаг гасит stdout), оставив пустой или JSON-less лог, но выйдя с кодом 0 — раньше это было неотличимо от успеха (`done` + auto-advance по пустому результату). Теперь `_monitor_agent` вызывает `_validate_result(output_path)`: +- лог отсутствует / пустой (0 байт или только whitespace) ⇒ невалиден; +- нет парсящегося trailing result-JSON (тот же контракт, что usage-учёт — `usage._extract_last_json_object`) ⇒ невалиден; +- хелпер защитный (never-raise); при собственной ошибке — fail-safe в сторону провала. + +`success = (exit_code==0 AND result_ok)`. Реальный `exit_code` пишется в `agent_runs` без искажения; на решение done/fail влияет отдельный флаг `result_ok` (не подменённый код выхода). Только при `success`: постится «успешный» status-коммент и вызывается `_try_advance_stage`. При `exit_code==0 AND not result_ok`: шлётся Telegram-алерт о пустом/невалидном результате, стадия НЕ двигается, а `_finalize_job(result_ok=False)` маршрутизирует job в провал (`empty run log / no result JSON`): по умолчанию permanent (`attempts mark_job done - exit !=0 & attempts requeue (queued) - exit !=0 & attempts>=max -> failed + Telegram + _finalize_job(result_ok): + exit 0 & result_ok -> mark_job done + else (exit!=0 ИЛИ пустой результат): + attempts requeue (queued) + attempts>=max -> failed + Telegram ``` +> ORCH-044 (P3): `result_ok` отражает валидность run-лога (непустой + есть result-JSON). `exit 0` с пустым/невалидным результатом идёт в ветку провала, НЕ в `done` (см. §4 «Валидация результата»). + ### Таблица `jobs` | Колонка | Назначение | diff --git a/docs/operations/INFRA.md b/docs/operations/INFRA.md index 90bd8e0..f2dac6c 100644 --- a/docs/operations/INFRA.md +++ b/docs/operations/INFRA.md @@ -54,6 +54,9 @@ | `ORCH_AGENT_EFFORT_DEFAULT` | режим работы `--effort` по умолчанию (ORCH-41): low\|medium\|high\|xhigh\|max; дефолт `high` | | `ORCH_AGENT_EFFORT_` | per-agent effort; дефолт: думающие → high, tester/deployer → medium | | `ORCH_AGENT_FALLBACK_MODEL` | опц. фолбэк-модель при overloaded (`--fallback-model`); пусто → без флага | +| `ORCH_PREFLIGHT_CHECK_AUTH` | вкл/выкл token-free auth-проверку preflight (ORCH-044); дефолт `true`. Аварийный тумблер: `false` → preflight как до ORCH-044 (только `--version`) | +| `ORCH_CLAUDE_CREDENTIALS_PATH` | явный путь к `.credentials.json` (ORCH-044); пусто → `/.claude/.credentials.json`, где `AGENT_HOME=/home/slin` — HOME, под которым launcher реально спавнит claude (не HOME процесса орка) | +| `ORCH_AUTH_EXPIRY_SKEW_SECONDS` | запас на рассинхрон часов при сравнении `claudeAiOauth.expiresAt` (ORCH-044); дефолт `0` | | `DEPLOY_SSH_USER` / `_HOST` / `DEPLOY_HOOK_SCRIPT` | параметры деплой-хука | **Секреты — только в `.env` / `.env.staging` на хосте, в гит НЕ коммитятся.** Канон — `.env.example`, `.env.staging.example`. @@ -81,6 +84,19 @@ > ⚠️ Бюджет (ORCH-38): `claude-opus-4-8` дефолт в коде; реальное переключение прод-env делается отдельно после согласования. +## Preflight auth-гейт (`src/preflight.py`, ORCH-044) +`claude --version` отвечает успешно **даже когда claude разлогинен** (версия — локальная инфа), поэтому до ORCH-044 preflight был слеп к авторизации: разлогиненный инстанс клеймил job и тихо умирал с пустым логом, блокируя общую очередь всех проектов. + +ORCH-044 добавляет **token-free** проверку (без сети, без prompt-ping — BR-1): +1. **Проактивно (основной гейт):** после успешного `--version` читается `/.claude/.credentials.json` (путь — `ORCH_CLAUDE_CREDENTIALS_PATH` или дефолт от `AGENT_HOME=/home/slin`, **не** HOME процесса орка). Нет файла / битый JSON / нет `claudeAiOauth.accessToken` ⇒ `check()=(False, …)`. `claudeAiOauth.expiresAt` (epoch ms) `<= now + ORCH_AUTH_EXPIRY_SKEW_SECONDS` ⇒ протух ⇒ FAIL. Нет `expiresAt` ⇒ OK (не плодим ложные срабатывания). Результат кешируется тем же `ORCH_PREFLIGHT_CACHE_TTL`, что и `--version`. +2. **Постфактум (защитная сетка):** если агент всё же стартовал при протухшей сессии, launcher детектит маркер (`not logged in` / `please run /login` / `unauthorized` / `401`) в run-логе и сбрасывает preflight-кеш, чтобы следующий тик переоценил auth. Auth-провал **не** считается transient и **не** крутит circuit breaker — гейт здесь preflight. + +При `auth=fail` job **не клеймится** (`_drain_once` уже корректен при `ok=False`), reason виден в `/queue` (`preflight_reason`). Аварийный тумблер `ORCH_PREFLIGHT_CHECK_AUTH=false` возвращает version-only поведение. + +> ⚠️ Риск ложноположительного auth-fail (R-1): неверный путь к credentials заблокирует клейм **всех** проектов (общая очередь). Митигация: единый источник `AGENT_HOME`, тумблер, обязательная проверка на staging (8501) перед прод-деплоем. ADR — `docs/work-items/ORCH-044/06-adr/ADR-001-preflight-auth-and-empty-result-failure.md`. + +> ℹ️ `--effort` (P2) в ORCH-044 **не трогается** — вынесен в ORCH-50. + ## ⚠️ Self-hosting — оркестратор дорабатывает САМ СЕБЯ **Факт:** прод-инстанс `orchestrator` (8500) — ОДИН на ВСЕ прод-проекты (enduro-trails + orchestrator), с ОБЩЕЙ БД `./data/orchestrator.db` и общей очередью задач (ORCH-1). diff --git a/docs/work-items/ORCH-044/00-business-request.md b/docs/work-items/ORCH-044/00-business-request.md new file mode 100644 index 0000000..4c72ce0 --- /dev/null +++ b/docs/work-items/ORCH-044/00-business-request.md @@ -0,0 +1,7 @@ +# Business Request: Надёжность запуска агента: preflight ловит auth+битый флаг, --effort фикс + +Work Item ID: ORCH-044 + +## Description + +TBD diff --git a/docs/work-items/ORCH-044/01-brd.md b/docs/work-items/ORCH-044/01-brd.md new file mode 100644 index 0000000..54697cb --- /dev/null +++ b/docs/work-items/ORCH-044/01-brd.md @@ -0,0 +1,90 @@ +# 01 — Business Requirements Document (BRD) + +**Work Item:** ORCH-044 +**Title:** Надёжность запуска агента: preflight ловит auth+битый флаг, --effort фикс +**Приоритет:** Высокий (надёжность конвейера) +**Автор запроса:** Слава, 05.06 («почему перед стартом аналитика не прошла проверка?») + +## 1. Контекст и инцидент (05.06) +Задача **ORCH-17** застряла на стадии Analysis ~30 минут. Аналитик-агент стартовал и +мгновенно «умирал»: run-лог — **пустой файл (0 байт)**, а job в очереди оставался в +состоянии `running` (вечное зависание без сигнала). + +Корневые причины (две, наложились): +1. **`claude` Not logged in** после ребилда контейнера — токен/сессия не поднялись. +2. **Флаг `--effort`** в связке с `--print --output-format json` (CLI 2.1.142) **гасил весь + stdout** — claude завершался с пустым выводом. + +**Главная системная проблема:** preflight-проверка пропустила обе битые задачи в работу — +она слепа к авторизации и не ловит «битый флаг → пустой вывод». + +## 2. Проблема (как есть) +- **P1. Дыра в preflight (главное).** `src/preflight.py` сознательно проверяет только + (a) `os.path.exists(CLAUDE_BIN)` и (b) `claude --version` (timeout 5s, без токенов). + Но `--version` отвечает успешно **даже когда claude НЕ залогинен** (версия — локальная + информация). Итог: `preflight=ok`, а реальный запуск падает `Not logged in`. Preflight + слеп к авторизации и пропускает заведомо нерабочие задачи в очередь. +- **P2. `--effort` ломает вывод.** Флаг `--effort ` совместно с + `--print`/`--output-format json` в CLI 2.1.142 даёт **пустой stdout** — агент молча + умирает. Сейчас effort **отключён в проде** хотфиксом (`.env`: `ORCH_AGENT_EFFORT_*=""`), + но дефолты в `src/config.py` всё ещё `high`/`medium`, а документация (INFRA.md, + internals.md, ORCH-41) описывает effort как рабочую фичу. Несоответствие кода/доков/прода. +- **P3. Пустой лог ≠ провал.** Агент с пустым run-логом (0 байт) и `exit 0` трактуется как + **успех** (`_finalize_job` → `done`, авто-advance стадии) либо вечно висит `running`. + Ни watchdog, ни ретрай не срабатывают. Нет сигнала об инциденте. + +## 3. Бизнес-последствия +- Любой сбой авторизации или несовместимости флага → **тихое зависание** задачи без алерта. +- Блокируется конвейер **всех** проектов (общий инстанс/очередь, self-hosting) — как было с + ORCH-17 (30 мин простоя, ручное вмешательство). +- Деградация доверия к автономности оркестратора: «проверка перед стартом» не работает. + +## 4. Цель +Сделать запуск агента **отказоустойчивым по входу и по выходу**: +1. Preflight ловит отсутствие/протухание авторизации **дёшево и без траты токенов** до того, + как job будет заклеймлен. +2. Разобраться с `--effort` и привести код/доки/прод к одному непротиворечивому состоянию. +3. Пустой/невалидный результат запуска трактуется как **провал** (job → `failed`), чтобы + сработали watchdog/ретрай и алерт, а не вечное зависание. + +## 5. Заинтересованные стороны +- **Owner/Слава** — инициатор, требует «проверки перед стартом». +- **Все проекты на инстансе** (enduro-trails и self-hosting ORCH) — страдают от простоя. +- **Агенты конвейера** — analyst/architect/... — все запускаются через единый launcher. + +## 6. Объём (Scope) +**В объёме:** +- Дешёвая token-free проверка авторизации в preflight. +- Расследование и решение по `--effort` (вернуть корректно ИЛИ задокументировать как + unsupported и убрать из кода/дефолтов/доков). +- Детекция «пустой лог / нет валидного result-JSON» как провала job с корректным + переводом в `failed` и срабатыванием ретрая/алерта. +- Обновление документации (INFRA.md / internals.md / CHANGELOG) в том же PR. + +**Вне объёма:** +- Prompt-ping (ping→pong) — **запрещено** (жжёт rate limit). Только локальные/дешёвые проверки. +- Реформа circuit breaker / backoff-логики (используем существующие механизмы). +- Изменение схемы стадий/конвейера. +- Автоматический re-login claude (восстановление авторизации) — отдельная задача. + +## 7. Бизнес-правила +- BR-1: Preflight **не тратит токены** и не делает сетевых вызовов к API модели. +- BR-2: Протухшая/нечитаемая авторизация → `preflight=fail` → job **не клеймится** (остаётся + `queued`), пишется warning, при необходимости — алерт/брейкер. +- BR-3: Пустой run-лог ИЛИ отсутствие валидного result-JSON при `exit 0` → job `failed` + (никогда не `done` и не вечный `running`). +- BR-4: Никаких `--no-verify`/обхода хуков без явного одобрения Owner. +- BR-5: Код, дефолты `config.py`, прод `.env` и документация по `--effort` должны быть + взаимно непротиворечивы после задачи. + +## 8. Критерии успеха (бизнес-уровень) +- Симуляция «не залогинен» → preflight ловит до клейма, job не стартует впустую. +- Симуляция «пустой лог + exit 0» → job становится `failed`, срабатывает ретрай/алерт. +- Состояние `--effort` однозначно: либо работает с json-форматом, либо удалён из активного + пути и доков (без «мёртвого» флага в дефолтах). +- Инцидент класса ORCH-17 больше не приводит к тихому 30-минутному зависанию. + +## 9. Связанные материалы +- `src/preflight.py`, `src/queue_worker.py`, `src/agents/launcher.py`, `src/config.py` +- `docs/history/LESSONS_ORCH-017.md`, `docs/history/LESSONS_2026-06-05.md` +- ORCH-41 (effort/model resolver), ORCH-1 (очередь/resilience), ORCH-7 (watchdog) diff --git a/docs/work-items/ORCH-044/02-trz.md b/docs/work-items/ORCH-044/02-trz.md new file mode 100644 index 0000000..09c6bb9 --- /dev/null +++ b/docs/work-items/ORCH-044/02-trz.md @@ -0,0 +1,143 @@ +# 02 — Техническое задание (ТЗ) + +**Work Item:** ORCH-044 +**Основано на:** 01-brd.md + +> Примечание: ТЗ фиксирует **что** должно измениться и **наблюдаемое поведение**. +> Выбор конкретной реализации (например, формат проверки `.credentials.json` vs парсинг +> маркера в логе) — за архитектором (стадия architecture, ADR). Где описаны варианты — +> это границы допустимого решения, а не предписание. + +> ## ⛔ КОРРЕКЦИЯ SCOPE ВЛАДЕЛЬЦЕМ (Слава, 06.06) — ЧИТАТЬ ПЕРВЫМ +> +> **P2 (`--effort`) ПОЛНОСТЬЮ ИСКЛЮЧЁН из этой задачи.** Решение владельца: +> - effort **НУЖЕН и работает** — его **НЕЛЬЗЯ** убирать как unsupported. **Вариант B запрещён.** +> - В ORCH-044 **НЕ трогать** `--effort`: ни `_spawn` effort_flag, ни `resolve_agent_effort`, ни дефолты `agent_effort_*` в `config.py`, ни ORCH-41 effort-доки. +> - Текущий прод-хотфикс `ORCH_AGENT_EFFORT_*=""` в `.env` **оставить как есть** — не снимать, не менять. +> - Полноценный возврат effort (расследование флагов + json) вынесен в **ОТДЕЛЬНУЮ задачу ORCH-50** («Эффорт агентов: заставить --effort работать с --print/json»). Туда же — любое расследование причины пустого stdout. +> +> **Архитектор/дев игнорируют все TR-2.x и AC-7/AC-8/AC-9, относящиеся к effort.** Реализуем ТОЛЬКО: +> - **P1** — preflight ловит auth (ОБА подхода: проактивно cred-файл `expiresAt` + постфактум маркер `Not logged in`); +> - **P3** — пустой лог / нет result-JSON ⇒ job `failed` (не `done`, не вечный `running`). +> +> Заголовок задачи содержит «--effort фикс» по историческим причинам — это НЕ часть scope. Effort = ORCH-50. + +## 1. Задействованные модули `src/` +| Модуль | Текущее место | Изменение | +|--------|---------------|-----------| +| `src/preflight.py` | `_run_version`, `_compute`, `check` | Добавить дешёвую token-free проверку авторизации (P1) | +| `src/config.py` | блок ORCH-41 effort (стр. 98–108), новый блок настроек preflight-auth | Настройки auth-проверки; решение по effort-дефолтам (P2) | +| `src/agents/launcher.py` | `_spawn` (effort_flag, стр. 290–292, 303–311), `_monitor_agent` (стр. 460–615), `_finalize_job` (стр. 630–667) | Решение по `--effort` (P2); детекция пустого лога / отсутствия result-JSON (P3) | +| `src/queue_worker.py` | `_drain_once` claim-gating (стр. 158–165) | Учесть новый auth-fail preflight в гейтинге клейма (P1) — при необходимости | +| `src/db.py` | `mark_job` | Использование существующего перевода job → `failed` (P3); новых колонок не требуется | + +Новых файлов модулей не предполагается обязательно; допускается выделение хелпера +(например, `_check_auth()` в `preflight.py`) — на усмотрение архитектора. + +## 2. Требования по проблемам + +### P1 — Preflight ловит авторизацию (token-free) +- **TR-1.1.** Preflight ДОЛЖЕН, помимо `os.path.exists(bin)` и `claude --version`, выполнять + **дешёвую проверку авторизации без обращения к API модели и без prompt-ping**. +- **TR-1.2.** Допустимые подходы (выбор — за архитектором, ADR): + - (a) Проверка существования и читаемости файла учётных данных + `~/.claude/.credentials.json` (HOME агента — `/home/slin`, см. launcher env, стр. 326) + и валидности OAuth-токена по дате истечения внутри + (`claudeAiOauth.expiresAt`, epoch ms) — `expiresAt <= now` ⇒ протух ⇒ fail; + - (b) Парсинг реального run-вывода на маркер `Not logged in` (и подобные) с переводом + job в провал и размыканием/учётом circuit breaker. + - Подход (a) предпочтителен как **проактивный** (ловит ДО клейма); (b) — как защитная + сетка постфактум. Допускается комбинация. +- **TR-1.3.** Путь к файлу учётных данных ДОЛЖЕН резолвиться согласованно с тем HOME, + под которым launcher реально спавнит claude (`/home/slin`), а не из окружения процесса + оркестратора (аналогично тому, как `_claude_bin()` следует за реально исполняемым путём). +- **TR-1.4.** Результат auth-проверки кешируется тем же механизмом, что и version-check + (`preflight_cache_ttl`), чтобы не читать файл на каждый тик воркера. +- **TR-1.5.** При `auth=fail`: `check()` возвращает `(False, reason)` с **информативным + reason** (например, `claude not logged in: credentials missing` / `OAuth token expired at + `). Job НЕ клеймится (поведение `_drain_once` уже корректно при `ok=False`). +- **TR-1.6.** Граница ответственности: preflight остаётся **локальным** (BR-1). Сетевая + валидация токена у провайдера — вне объёма. +- **TR-1.7.** Поведение при «всё хорошо» не меняется: залогинен + валидный токен ⇒ `ok=True`. + +### P2 — Решение по `--effort` +- **TR-2.1.** Провести расследование (стадия architecture/development): причина пустого + stdout при `--effort` + `--print --output-format json` в CLI 2.1.142 — несовместимость + с json-форматом, иной синтаксис флага, или баг CLI. Зафиксировать вывод в ADR/`10-tech-risks.md`. +- **TR-2.2.** По итогам выбрать **ровно один** исход и привести к нему код+доки+дефолты: + - **Вариант A (вернуть effort):** найден корректный способ (например, иной синтаксис или + несовместимость только с конкретным output-format) — `--effort` снова формируется в + `_spawn` корректно; прод-хотфикс `ORCH_AGENT_EFFORT_*=""` снимается; добавить + регресс-тест, что вывод не пустой. + - **Вариант B (unsupported):** effort несовместим — **убрать `--effort` из активного пути + запуска** (`_spawn` не формирует `effort_flag`), убрать/нейтрализовать дефолты effort в + `config.py`, обновить ORCH-41-доки (INFRA.md, internals.md) пометив фичу как unsupported + на данной версии CLI. `resolve_agent_effort` либо удаляется, либо документированно + оставляется заглушкой (решение — ADR). +- **TR-2.3.** Независимо от A/B: **не должно остаться «мёртвого» флага**, который тихо гасит + вывод. После задачи запуск с дефолтной конфигурацией прода ДОЛЖЕН давать непустой + result-JSON. +- **TR-2.4.** Изменение дефолтов/удаление флага не должно ломать `resolve_agent_model` + (модель — независимая фича ORCH-41) и существующие тесты `test_resolve_agent_effort.py` + (их допустимо обновить под новый контракт). + +### P3 — Пустой лог / нет result-JSON ⇒ провал +- **TR-3.1.** В `_monitor_agent`/`_finalize_job`: при `exit_code == 0` ДОЛЖНА выполняться + **проверка валидности результата** перед тем как считать job успешным: + - run-лог **непустой** (размер > 0 и/или содержит непустой текст), И + - из него извлекается **валидный result-JSON** (тот же контракт, что использует + `usage._extract_last_json_object` / `parse_usage_from_log`). +- **TR-3.2.** Если результат невалиден (пустой лог ИЛИ нет валидного JSON) при `exit_code==0`, + job ДОЛЖЕН трактоваться как **провал**: + - НЕ переводиться в `done`; + - попасть в путь ретрая/провала (`attempts < max_attempts` ⇒ requeue, иначе `failed`), + аналогично permanent-ветке `_finalize_permanent`, с информативным `error` + (например, `empty run log / no result JSON (run_id=...)`); + - сгенерировать алерт (Telegram), как прочие провалы; + - НЕ выполнять авто-advance стадии (`_try_advance_stage`) и НЕ постить «успешный» + status-коммент. +- **TR-3.3.** Классификация такого провала: по умолчанию — **permanent** (это не 429/overload). + Если в логе присутствует transient-маркер (через `error_classifier`) — допускается + transient-путь. Auth-провал (`Not logged in`) — на усмотрение архитектора: может + маршрутизироваться как сигнал брейкеру (P1/TR-1.2b). +- **TR-3.4.** Никогда не оставлять job в `running` навечно из-за пустого результата: либо + `done` (валидно), либо `failed`/`queued`(retry). (Watchdog ORCH-7 продолжает закрывать + случай таймаута; здесь закрывается случай «быстрая смерть с exit 0».) +- **TR-3.5.** Защитность: вся проверка обёрнута так, что её собственная ошибка не роняет + монитор (как и прочий код `_monitor_agent`); при сомнении — fail-safe в сторону провала job. + +## 3. Изменения API +Нет новых/изменённых HTTP-endpoint'ов. Допускается обогащение поля `preflight_reason` в +`/queue` (через существующий `worker.status()` / `QueueWorker.last_preflight_reason`) более +информативным auth-сообщением — без изменения схемы ответа. + +## 4. Изменения схемы БД +Нет. Используются существующие колонки `jobs` (`status`, `error`, `attempts`, +`max_attempts`, `transient_attempts`) и `agent_runs`. Новых таблиц/колонок не требуется. + +## 5. Требования к новым QG checks +Новых Quality Gate проверок не требуется — изменения в слое запуска/preflight, не в гейтах +стадий. Реестр `QG_CHECKS` не меняется. + +## 6. Конфигурация (env / config.py) +- Возможные новые настройки preflight-auth (имена — на усмотрение архитектора), например: + - `ORCH_PREFLIGHT_CHECK_AUTH` (bool, default true) — включение auth-проверки; + - путь к credentials, если не выводится из HOME автоматически. +- Решение по effort-дефолтам (`agent_effort_*`) согласно TR-2.2 (нейтрализовать при варианте B). +- Все новые настройки документируются в `config.py` docstring и в INFRA.md (env-карта). + +## 7. Артефакты pipeline (обязательны к созданию/обновлению) +- `06-adr/ADR-NNN-*.md` — решение по подходу preflight-auth (a/b/комбо) и по effort (A/B). +- `10-tech-risks.md` — риск ложноположительной auth-проверки, риск регрессии effort, риск + fail-safe-провала на легитимных пустых выводах. +- `12-review.md`, `13-test-report.md` — по стадиям. +- Обновить `docs/operations/INFRA.md` и `docs/architecture/internals.md` (effort-секции), + `CHANGELOG.md`. Документация = golden source (правило агентов №2). + +## 8. Ограничения и запреты +- ❌ Prompt-ping в preflight (жжёт rate limit) — запрещено (BR-1, комментарий в preflight.py). +- ❌ Сетевые вызовы к API модели в preflight. +- ❌ Оставлять job в `running` без таймаута при пустом результате. +- ❌ `--no-verify`/обход хуков без одобрения Owner. +- ⚠️ Self-hosting: не ронять прод-контейнер `orchestrator`; проверка изменений — через + staging (8501) перед прод-деплоем (см. CLAUDE.md, INFRA.md). diff --git a/docs/work-items/ORCH-044/03-acceptance-criteria.md b/docs/work-items/ORCH-044/03-acceptance-criteria.md new file mode 100644 index 0000000..4e6da4b --- /dev/null +++ b/docs/work-items/ORCH-044/03-acceptance-criteria.md @@ -0,0 +1,122 @@ +# 03 — Критерии приёмки (Acceptance Criteria) + +**Work Item:** ORCH-044 +Каждый критерий — однозначное PASS/FAIL. Привязка к TR из `02-trz.md`. + +## P1 — Preflight ловит авторизацию + +### AC-1 — Не залогинен ⇒ preflight FAIL (TR-1.1, TR-1.2, TR-1.5) +- **Дано:** бинарь claude существует, `claude --version` отвечает успешно, НО учётные + данные отсутствуют/нечитаемы (нет `.credentials.json`). +- **Когда:** вызывается `preflight.check(force=True)`. +- **Тогда:** возвращается `(False, reason)`, где `reason` упоминает авторизацию + (например, «not logged in» / «credentials»). +- **FAIL если:** возвращается `(True, ...)` (как сейчас — слепота к auth). + +### AC-2 — Протухший OAuth-токен ⇒ preflight FAIL (TR-1.2a) +- **Дано:** `.credentials.json` существует и читаем, но `claudeAiOauth.expiresAt` в прошлом. +- **Когда:** `preflight.check(force=True)`. +- **Тогда:** `(False, reason)` с указанием на истечение токена. +- *(N/A, если архитектор выбрал чистый вариант (b) без чтения файла — тогда покрывается AC-9.)* + +### AC-3 — Валидный логин ⇒ preflight OK без регрессии (TR-1.7) +- **Дано:** bin есть, `--version` ок, `.credentials.json` читаем, `expiresAt` в будущем. +- **Когда:** `preflight.check(force=True)`. +- **Тогда:** `(True, ...)`. +- **FAIL если:** залогиненный валидный кейс даёт FAIL (ложное срабатывание). + +### AC-4 — Auth-fail блокирует клейм job (TR-1.5, BR-2) +- **Дано:** preflight возвращает `(False, ...)` из-за auth; в очереди есть `queued` job. +- **Когда:** `QueueWorker._drain_once()` выполняет тик. +- **Тогда:** job **не клеймится** (остаётся `queued`), в `worker.last_preflight_ok=False`, + пишется лог-warning; claude не спавнится. +- **FAIL если:** job переходит в `running` / спавнится агент. + +### AC-5 — Token-free и локально (BR-1, TR-1.6) +- **Дано:** auth-проверка. +- **Тогда:** она НЕ делает prompt-ping и НЕ обращается к API модели (никаких httpx/сетевых + вызовов к провайдеру в пути проверки; проверяется по коду/моку — сетевой вызов не + происходит). +- **FAIL если:** проверка отправляет запрос к модели/жжёт токены. + +### AC-6 — Кеширование auth-проверки (TR-1.4) +- **Дано:** `preflight_cache_ttl` > 0, первый `check()` выполнен. +- **Когда:** повторные `check()` в пределах TTL. +- **Тогда:** дорогая часть (чтение файла/процесс) не повторяется чаще TTL (как у version-check). +- **FAIL если:** файл/процесс дёргается на каждый тик внутри TTL. + +## P2 — Решение по `--effort` + +> ⛔ **ИСКЛЮЧЕНО ВЛАДЕЛЬЦЕМ (06.06):** AC-7, AC-8, AC-9 НЕ применяются в ORCH-044. effort не трогаем, вынесен в ORCH-50. См. коррекцию scope в 02-trz.md. + + +### AC-7 — Расследование задокументировано (TR-2.1) +- **Тогда:** в ADR (`06-adr/`) и/или `10-tech-risks.md` зафиксирована причина пустого stdout + при `--effort` + `--print --output-format json` (несовместимость/синтаксис/баг CLI). +- **FAIL если:** изменения внесены без объяснения первопричины. + +### AC-8 — Однозначный исход A или B, без «мёртвого» флага (TR-2.2, TR-2.3) +- **Тогда:** реализован ровно один из вариантов: + - **A:** `--effort` формируется и запуск с ним даёт **непустой** result-JSON; прод-хотфикс + `ORCH_AGENT_EFFORT_*=""` более не требуется; есть регресс-тест на непустой вывод; ИЛИ + - **B:** `--effort` **не формируется** в активном пути `_spawn`; дефолты `agent_effort_*` + нейтрализованы; ORCH-41-доки помечают effort как unsupported на текущем CLI. +- **FAIL если:** в коде остаётся путь, где дефолтная конфигурация добавляет `--effort` и + гасит вывод; ИЛИ код/доки/дефолты противоречат друг другу. + +### AC-9 — Дефолтный запуск даёт непустой результат (TR-2.3, перекликается с P3) +- **Дано:** конфигурация по умолчанию после задачи (без ручного хотфикса в `.env`). +- **Когда:** агент запускается стандартным путём `_spawn`. +- **Тогда:** результат запуска — непустой run-лог с валидным result-JSON (проверяемо + модульно через построение cmd и/или интеграционно на моке claude). +- **FAIL если:** дефолтный путь воспроизводит пустой stdout инцидента. + +## P3 — Пустой лог / нет result-JSON ⇒ провал + +### AC-10 — Пустой лог + exit 0 ⇒ job НЕ done (TR-3.1, TR-3.2) +- **Дано:** агент завершился `exit_code=0`, но run-лог пустой (0 байт). +- **Когда:** отрабатывает `_monitor_agent`/`_finalize_job`. +- **Тогда:** job НЕ переходит в `done`; переходит в `failed` (или `queued` при наличии + retry-бюджета) с информативным `error`; шлётся алерт. +- **FAIL если:** job становится `done`, либо остаётся `running` навсегда. + +### AC-11 — Нет валидного result-JSON + exit 0 ⇒ job НЕ done (TR-3.1, TR-3.2) +- **Дано:** run-лог непустой, но не содержит валидного result-JSON (мусор/обрезок). +- **Когда:** финализация job. +- **Тогда:** job трактуется как провал (как AC-10). +- **FAIL если:** job становится `done`. + +### AC-12 — Нет авто-advance и нет «успешного» коммента при провале результата (TR-3.2) +- **Дано:** кейс AC-10/AC-11. +- **Тогда:** `_try_advance_stage` НЕ вызывается (стадия не двигается), «успешный» + status-коммент агента НЕ постится. +- **FAIL если:** стадия продвинулась/запостился успех при пустом результате. + +### AC-13 — Валидный результат не регрессирует (TR-3.1) +- **Дано:** `exit_code=0` и непустой run-лог с валидным result-JSON. +- **Когда:** финализация job. +- **Тогда:** job → `done`, авто-advance и usage-коммент работают как раньше. +- **FAIL если:** легитимный успешный запуск теперь ошибочно помечается провалом. + +### AC-14 — Никогда не вечный `running` (TR-3.4, BR-3) +- **Тогда:** для любого завершившегося процесса (любой exit_code, включая 0 с пустым логом) + job завершается в терминальном/ретраябельном состоянии (`done`/`failed`/`queued`), не + остаётся `running`. +- **FAIL если:** существует путь, оставляющий job `running` после выхода процесса. + +## Сквозные + +### AC-15 — Документация обновлена в том же PR (правило агентов №2, №6) +- **Тогда:** обновлены `docs/operations/INFRA.md` (env-карта preflight-auth и/или effort), + `docs/architecture/internals.md` (effort-секция), `CHANGELOG.md`; заведён ADR. +- **FAIL если:** функционал изменён, доки/CHANGELOG/ADR не обновлены (reviewer → REQUEST_CHANGES). + +### AC-16 — Тесты зелёные (test-plan) +- **Тогда:** все тесты из `04-test-plan.yaml` проходят; `pytest tests/ -q` зелёный. +- **FAIL если:** хотя бы один тест плана FAIL или существующие тесты сломаны без обоснованного + обновления контракта. + +### AC-17 — Self-hosting безопасность (CLAUDE.md) +- **Тогда:** изменения не требуют рестарта/падения прод-контейнера `orchestrator` в рамках + задачи; проверка прошла через staging (8501). +- **FAIL если:** задача ломает/рестартует прод-инстанс, останавливая конвейер других проектов. diff --git a/docs/work-items/ORCH-044/04-test-plan.yaml b/docs/work-items/ORCH-044/04-test-plan.yaml new file mode 100644 index 0000000..517ab08 --- /dev/null +++ b/docs/work-items/ORCH-044/04-test-plan.yaml @@ -0,0 +1,145 @@ +work_item: ORCH-044 +title: "Надёжность запуска агента: preflight auth + --effort фикс + пустой лог = провал" +notes: > + Реальный claude/Popen НЕ спавнится: subprocess и launcher мокаются (паттерн + tests/test_resilience.py). БД — свежий per-test sqlite (fixture fresh_db). + Файлы учётных данных создаются во временном каталоге (tmp_path) и путь + мокается. Сетевые вызовы запрещены — проверяются моками/отсутствием httpx. + +tests: + # ---------------- P1: preflight ловит авторизацию ---------------- + - id: TC-01 + type: unit + description: "Нет .credentials.json при рабочем --version -> preflight.check() = (False, reason про auth)" + module: tests/test_preflight_auth.py + covers: [AC-1, TR-1.1, TR-1.2] + expected: PASS + + - id: TC-02 + type: unit + description: "Протухший OAuth (claudeAiOauth.expiresAt в прошлом) -> preflight FAIL про истечение токена" + module: tests/test_preflight_auth.py + covers: [AC-2, TR-1.2a] + expected: PASS + + - id: TC-03 + type: unit + description: "Валидный логин (credentials читаемы, expiresAt в будущем) -> preflight OK, без регрессии" + module: tests/test_preflight_auth.py + covers: [AC-3, TR-1.7] + expected: PASS + + - id: TC-04 + type: unit + description: "Нечитаемый/битый .credentials.json (невалидный JSON) -> preflight FAIL, не падает исключением" + module: tests/test_preflight_auth.py + covers: [AC-1, TR-1.2a, TR-3.5] + expected: PASS + + - id: TC-05 + type: unit + description: "Auth-проверка token-free: при check() не происходит сетевого вызова к API модели (мок httpx/urlopen не вызван)" + module: tests/test_preflight_auth.py + covers: [AC-5, BR-1, TR-1.6] + expected: PASS + + - id: TC-06 + type: unit + description: "Auth-результат кешируется: повторные check() в пределах preflight_cache_ttl не перечитывают credentials" + module: tests/test_preflight_auth.py + covers: [AC-6, TR-1.4] + expected: PASS + + - id: TC-07 + type: unit + description: "Путь к credentials резолвится от HOME агента (/home/slin), а не от окружения процесса оркестратора" + module: tests/test_preflight_auth.py + covers: [TR-1.3] + expected: PASS + + - id: TC-08 + type: integration + description: "QueueWorker._drain_once при preflight auth-fail не клеймит job: job остаётся queued, claude не спавнится, last_preflight_ok=False" + module: tests/test_preflight_auth.py + covers: [AC-4, BR-2, TR-1.5] + expected: PASS + + # ---------------- P2: решение по --effort ---------------- + - id: TC-09 + type: unit + description: "Вариант B: при дефолтной конфигурации построенная cmd в _spawn НЕ содержит '--effort' (флаг не гасит вывод). При варианте A — тест адаптируется на корректное формирование effort" + module: tests/test_effort_flag.py + covers: [AC-8, TR-2.2, TR-2.3] + expected: PASS + + - id: TC-10 + type: unit + description: "resolve_agent_effort согласован с принятым решением (B: нейтрализован/пусто по дефолту; A: валидное значение). Существующий test_resolve_agent_effort обновлён под новый контракт и зелёный" + module: tests/test_resolve_agent_effort.py + covers: [AC-8, TR-2.4] + expected: PASS + + - id: TC-11 + type: integration + description: "Дефолтный путь запуска (мок claude, отдающий валидный result-JSON) даёт непустой лог с валидным JSON — воспроизведение инцидента (пустой stdout) не происходит" + module: tests/test_effort_flag.py + covers: [AC-9, TR-2.3] + expected: PASS + + # ---------------- P3: пустой лог / нет result-JSON = провал ---------------- + - id: TC-12 + type: integration + description: "exit_code=0 + пустой run-лог (0 байт) -> job НЕ done; помечается failed (или queued при retry-бюджете) с информативным error; алерт вызван" + module: tests/test_empty_log_failure.py + covers: [AC-10, TR-3.1, TR-3.2] + expected: PASS + + - id: TC-13 + type: integration + description: "exit_code=0 + лог без валидного result-JSON (мусор) -> job трактуется как провал, не done" + module: tests/test_empty_log_failure.py + covers: [AC-11, TR-3.1] + expected: PASS + + - id: TC-14 + type: integration + description: "При провале по пустому результату _try_advance_stage НЕ вызывается и успешный usage-коммент НЕ постится" + module: tests/test_empty_log_failure.py + covers: [AC-12, TR-3.2] + expected: PASS + + - id: TC-15 + type: integration + description: "exit_code=0 + непустой лог с валидным result-JSON -> job done, авто-advance и usage-коммент работают (нет регрессии)" + module: tests/test_empty_log_failure.py + covers: [AC-13, TR-3.1] + expected: PASS + + - id: TC-16 + type: integration + description: "Любой выход процесса не оставляет job в 'running': пустой лог+exit0 завершается терминально (done/failed/queued)" + module: tests/test_empty_log_failure.py + covers: [AC-14, BR-3, TR-3.4] + expected: PASS + + - id: TC-17 + type: unit + description: "Классификация пустого результата по умолчанию permanent; transient-маркер в логе уводит в transient-путь (error_classifier)" + module: tests/test_empty_log_failure.py + covers: [TR-3.3] + expected: PASS + + # ---------------- Регрессия / сквозное ---------------- + - id: TC-18 + type: unit + description: "Регресс: существующие preflight-кейсы (bin missing, --version ok) из test_resilience.py остаются зелёными после добавления auth-слоя" + module: tests/test_resilience.py + covers: [AC-3, TR-1.7] + expected: PASS + + - id: TC-19 + type: integration + description: "Полный прогон 'pytest tests/ -q' зелёный — ни один существующий тест не сломан без обоснованного обновления контракта" + module: tests/ + covers: [AC-16] + expected: PASS diff --git a/docs/work-items/ORCH-044/06-adr/ADR-001-preflight-auth-and-empty-result-failure.md b/docs/work-items/ORCH-044/06-adr/ADR-001-preflight-auth-and-empty-result-failure.md new file mode 100644 index 0000000..24e133b --- /dev/null +++ b/docs/work-items/ORCH-044/06-adr/ADR-001-preflight-auth-and-empty-result-failure.md @@ -0,0 +1,168 @@ +# ADR-001: Token-free auth-preflight + «пустой результат = провал» в запуске агента + +**Work Item:** ORCH-044 +**Статус:** Accepted +**Дата:** 2026-06-06 +**Автор:** Architect + +> ⛔ **Scope (коррекция владельца, 06.06):** `--effort` (P2) **исключён** из ORCH-044 и +> вынесен в **ORCH-50**. Этот ADR покрывает только **P1** (preflight ловит авторизацию) +> и **P3** (пустой лог / нет result-JSON ⇒ job `failed`). Любые решения по effort, +> дефолтам `agent_effort_*` и ORCH-41 effort-докам — **вне этого ADR**. + +--- + +## Контекст + +Инцидент 05.06 (ORCH-17): аналитик-агент стартовал и мгновенно «умирал» — run-лог пустой +(0 байт), job в очереди завис в `running`. Две наложившиеся причины: (1) `claude Not logged +in` после ребилда контейнера; (2) `--effort` гасил stdout. **Системная проблема:** +preflight пропустил заведомо нерабочую задачу в работу, а пустой результат был неотличим +от успеха. Поскольку инстанс общий для всех проектов (self-hosting, общая очередь/БД), +тихое зависание блокирует конвейер **всех** проектов. + +Текущее состояние слоя запуска: +- `src/preflight.py` проверяет только `os.path.exists(bin)` и `claude --version`. `--version` + отвечает успешно **даже когда claude не залогинен** (версия — локальная информация) ⇒ + preflight слеп к авторизации. +- `src/agents/launcher.py::_monitor_agent` трактует `exit_code == 0` как успех **независимо + от формы stdout** (комментарий в `_spawn`, стр. 302) ⇒ пустой лог + exit 0 → `done` + + авто-advance стадии. + +Ограничения (BR-1): preflight обязан быть **локальным и token-free** — никакого prompt-ping +и сетевых вызовов к API модели. + +## Решение + +### P1 — Preflight ловит авторизацию (комбинация проактивной и постфактум-проверок) + +Реализуем **оба** подхода из TR-1.2 (a + b), проактивный — основной гейт, постфактум — +защитная сетка. + +**(a) Проактивно — чтение файла учётных данных (основной гейт).** +`preflight._compute()` после успешного `--version` выполняет `_check_auth()`: +1. Резолвит путь к credentials **согласованно с HOME, под которым launcher реально спавнит + claude** (`/home/slin`), а НЕ из окружения процесса оркестратора. Реализуется зеркально + `_claude_bin()`: новый `_agent_home()` читает `AgentLauncher.AGENT_HOME` (новая константа, + значение `/home/slin`), путь = `settings.claude_credentials_path` если задан, иначе + `/.claude/.credentials.json`. +2. Файла нет / нечитаем / невалидный JSON ⇒ `(False, "claude not logged in: credentials …")`. +3. Нет блока `claudeAiOauth` / accessToken ⇒ `(False, "not logged in: no oauth token")`. +4. `claudeAiOauth.expiresAt` (epoch **ms**) `<= now_ms (+ skew)` ⇒ + `(False, "OAuth token expired at ")`. +5. accessToken есть, но `expiresAt` отсутствует/не число ⇒ **OK** (нельзя доказать истечение; + не плодим ложные срабатывания — см. Риски). +6. Иначе ⇒ `(True, "auth ok")`. + +`_check_auth()` **никогда не бросает**: любое исключение → `(False, "auth check error: …")` +(fail-safe в сторону «не клеймить», BR-2 / TR-3.5). + +Кеширование (TR-1.4 / AC-6): чтение файла встроено в `_compute()`, который уже кешируется +`check()` на `preflight_cache_ttl`. **Отдельный кеш не вводится** — auth-чтение происходит +только на cache-miss, как и `--version`. + +Гейтинг клейма (TR-1.5 / AC-4 / BR-2): **изменений в `queue_worker._drain_once` не требуется** +— он уже не клеймит job при `ok=False`. Информативный auth-reason автоматически попадает в +`worker.last_preflight_reason` и `/queue` (без изменения схемы ответа). + +**(b) Постфактум — маркер `Not logged in` в run-логе (защитная сетка).** +Если агент всё-таки стартовал при протухшей сессии (гонка: токен истёк между preflight и +спавном), `launcher` при финализации детектит auth-маркер в логе +(`preflight.is_auth_failure_text(text)`: «not logged in», «please run /login», +«unauthorized», «401») и: +- включает маркер в `error` job; +- вызывает `preflight.reset_cache()`, чтобы **следующий тик воркера переоценил auth + проактивно** (быстрый подхват re-login ИЛИ дальнейшее гейтирование, если всё ещё битый). + +Auth-провал **не** маршрутизируется как transient (это не 429) и **не** крутит брейкер — +правильный механизм гейтирования здесь preflight, а не circuit breaker. + +### P3 — Пустой лог / нет result-JSON ⇒ провал job + +В `_monitor_agent` для ветки `exit_code == 0` вводим **валидацию результата** перед тем как +считать job успешным. Новый защитный хелпер `_validate_result(output_path) -> (ok, reason)`: +- лог отсутствует / пустой (size 0 или только whitespace) ⇒ невалиден; +- иначе извлекаем result-JSON **тем же контрактом**, что usage-учёт + (`usage._extract_last_json_object` / `parse_usage_from_text`); нет валидного объекта ⇒ + невалиден; +- хелпер обёрнут try/except и **не роняет монитор**; при собственной ошибке — + fail-safe в сторону провала (TR-3.5). + +`success = (exit_code == 0 and result_ok)`. Побочные эффекты успеха выполняются **только при +`success`**: +- `_post_usage_comments(...)` (успешный status-коммент) — **не** постится при невалидном + результате (AC-12); +- `_try_advance_stage(...)` — **не** вызывается при невалидном результате (AC-12); +- при `exit_code == 0 and not result_ok` шлётся Telegram-алерт о «пустом/невалидном + результате». + +Финализация job (`_finalize_job` получает новый флаг `result_ok`): +- `exit_code == 0 and result_ok` ⇒ `done` (как раньше, AC-13 — без регрессии); +- `exit_code != 0` **ИЛИ** `result_ok == False` ⇒ путь провала: + - классификация лога `error_classifier.classify_log_file` (по умолчанию **permanent**; + transient-маркер уводит в transient-путь — TR-3.3); + - permanent: `attempts < max_attempts` ⇒ requeue (`queued`), иначе `failed` + алерт; + - `error` информативен: `empty run log / no result JSON (run_id=…)` для случая пустого + результата. + +Реальный `exit_code` по-прежнему пишется в `agent_runs` без искажения; на решение +done/fail влияет отдельный флаг `result_ok`, а не подменённый код выхода. + +`exit_code == 0` теперь **всегда** завершается терминально/ретраябельно (`done` | +`failed` | `queued`) — путь «быстрая смерть с exit 0 → вечный running» закрыт (AC-14, BR-3). +Watchdog ORCH-7 продолжает закрывать таймауты. + +### Конфигурация (config.py) + +| Настройка | Env | Default | Назначение | +|-----------|-----|---------|------------| +| `preflight_check_auth` | `ORCH_PREFLIGHT_CHECK_AUTH` | `True` | Вкл/выкл auth-проверку (аварийный тумблер) | +| `claude_credentials_path` | `ORCH_CLAUDE_CREDENTIALS_PATH` | `""` | Явный путь; пусто ⇒ `/.claude/.credentials.json` | +| `auth_expiry_skew_seconds` | `ORCH_AUTH_EXPIRY_SKEW_SECONDS` | `0` | Запас на рассинхрон часов при сравнении `expiresAt` | + +`agent_effort_*` дефолты и `--effort` в `_spawn` — **не трогаем** (scope, ORCH-50). + +## Альтернативы + +- **A1. Prompt-ping (ping→pong) для проверки auth.** ❌ Запрещено BR-1 (жжёт rate limit, + латентность). Отвергнуто. +- **A2. Только постфактум-маркер (чистый вариант b).** Ловит auth лишь ПОСЛЕ спавна и траты + цикла; не гейтирует клейм. Оставлен как защитная сетка, но не как основной механизм. +- **A3. Сетевая валидация токена у провайдера.** Нарушает «preflight локальный» (TR-1.6), + добавляет сетевую зависимость в горячий путь воркера. Отвергнуто. +- **A4. Подменять exit_code на ненулевой при пустом результате.** Исказило бы + `agent_runs.exit_code` и классификацию. Выбрали отдельный флаг `result_ok`. +- **A5. Отдельный кеш для auth.** Избыточно — `_compute()` уже под общим TTL. + +## Последствия + +**Плюсы.** +- Заведомо нерабочая (не залогинен / протухший токен) задача **не клеймится** — экономия + цикла и отсутствие тихого зависания. +- Пустая «быстрая смерть» агента теперь видима: `failed`/retry + алерт вместо ложного `done` + и движения стадии вперёд по пустому результату. +- Без изменения схемы БД, без новых QG/стадий, без новых HTTP-endpoint'ов. +- Auth-reason виден в `/queue` для диагностики. + +**Минусы / ограничения.** +- **Риск ложноположительного auth-fail** (см. `10-tech-risks.md` R-1): неверно + резолвленный путь к credentials заблокирует клейм **всех** проектов (общая очередь). + Митигируется: единый источник HOME (`AGENT_HOME`), тумблер `ORCH_PREFLIGHT_CHECK_AUTH`, + обязательная проверка на staging (8501) перед прод-деплоем. +- Проверка `expiresAt` — локальная; реально отозванный, но ещё не истёкший токен ловится + только постфактум-маркером (b). +- `expiresAt`-отсутствие трактуется как OK (компромисс против ложных срабатываний). + +**Self-hosting.** Изменения только в слое preflight/launch; **не** требуют рестарта/падения +прод-контейнера `orchestrator` в рамках задачи. Выкатка — через staging-гейт (AC-17). + +## Связи + +- BRD `01-brd.md` (P1, P3), ТЗ `02-trz.md` (TR-1.x, TR-3.x; scope-коррекция), + Acceptance `03-acceptance-criteria.md` (AC-1…AC-6, AC-10…AC-17). +- Риски: `10-tech-risks.md`. Инфра: `07-infra-requirements.md`. БД: `08-data-requirements.md`. +- Код: `src/preflight.py`, `src/agents/launcher.py` (`_monitor_agent`, `_finalize_job`), + `src/config.py`, `src/usage.py` (`_extract_last_json_object`), + `src/error_classifier.py` (`classify_log_file`), `src/queue_worker.py` (без изменений). +- ORCH-1 (очередь/resilience), ORCH-7 (watchdog), ORCH-41 (resolver — **не трогаем effort**). +- **ORCH-50** — полноценный возврат `--effort` (вынесен из этой задачи). diff --git a/docs/work-items/ORCH-044/07-infra-requirements.md b/docs/work-items/ORCH-044/07-infra-requirements.md new file mode 100644 index 0000000..fe00da6 --- /dev/null +++ b/docs/work-items/ORCH-044/07-infra-requirements.md @@ -0,0 +1,46 @@ +# 07 — Требования к инфраструктуре + +**Work Item:** ORCH-044 +**Основано на:** ADR-001, ТЗ `02-trz.md` + +## Топология +**Без изменений.** Новых контейнеров, портов, сервисов, очередей не вводится. Прод +`orchestrator` (8500) и staging `orchestrator-staging` (8501) остаются как есть +(`docs/operations/INFRA.md`). + +## Учётные данные claude (P1) +- Launcher спавнит claude с `HOME=/home/slin` (`src/agents/launcher.py`). Preflight ДОЛЖЕН + резолвить путь к credentials от **этого же** HOME, а не от окружения процесса оркестратора. +- Ожидаемое расположение файла OAuth-токена: **`/home/slin/.claude/.credentials.json`** + (структура: `claudeAiOauth.expiresAt` — epoch **ms**). +- Файл — секрет; в гит НЕ коммитится (правило агентов №8). На хосте монтируется в контейнер + как раньше; задача его расположение **не меняет**, только начинает читать. +- ⚠️ **Проверить на staging:** реальный путь файла внутри контейнера совпадает с + резолвленным preflight. Несовпадение ⇒ ложный auth-fail и блок очереди (R-1). + +## Новые переменные окружения (env-карта) +Документировать в `docs/operations/INFRA.md` и docstring `src/config.py`: + +| Env | Default | Назначение | +|-----|---------|------------| +| `ORCH_PREFLIGHT_CHECK_AUTH` | `true` | Включение token-free auth-проверки в preflight. Аварийный тумблер: `false` возвращает старое поведение (только bin + `--version`). | +| `ORCH_CLAUDE_CREDENTIALS_PATH` | `""` | Явный путь к `.credentials.json`. Пусто ⇒ `/.claude/.credentials.json`. | +| `ORCH_AUTH_EXPIRY_SKEW_SECONDS` | `0` | Запас на рассинхрон часов при сравнении `expiresAt`. | + +`--effort` env (`ORCH_AGENT_EFFORT_*`) — **вне scope**; прод-хотфикс `ORCH_AGENT_EFFORT_*=""` +в `.env` **оставить как есть** (ORCH-50). + +## Эксплуатационные процедуры +- **Аварийный откат auth-гейта без редеплоя кода:** выставить `ORCH_PREFLIGHT_CHECK_AUTH=false` + в `.env` и перезапустить воркер обычной процедурой выката (НЕ в рамках этой задачи). +- **Диагностика:** auth-причина видна в `GET /queue` (`preflight_reason`) и в warning-логе + `orchestrator.preflight`. +- **Re-login:** при детекте auth-маркера в логе launcher сбрасывает preflight-кеш, поэтому + после ручного `claude /login` следующий тик воркера (≤ `preflight_cache_ttl`) подхватит + валидную сессию автоматически. + +## Self-hosting / деплой (AC-17) +- Изменения только в слое preflight/launch — **не** требуют рестарта/падения прод-контейнера + в рамках задачи. +- Выкатка self-доработки ORCH — **через staging-гейт (8501)** перед прод-деплоем + (CLAUDE.md, `docs/operations/INFRA.md`, ADR-0003). diff --git a/docs/work-items/ORCH-044/08-data-requirements.md b/docs/work-items/ORCH-044/08-data-requirements.md new file mode 100644 index 0000000..2e557e6 --- /dev/null +++ b/docs/work-items/ORCH-044/08-data-requirements.md @@ -0,0 +1,23 @@ +# 08 — Требования к схеме БД + +**Work Item:** ORCH-044 +**Основано на:** ADR-001, ТЗ `02-trz.md` §4 + +## Вердикт: изменений схемы НЕ требуется + +Новых таблиц, колонок, индексов, миграций — **нет**. + +P1 (auth-preflight) и P3 (пустой результат ⇒ провал) работают на **существующих** структурах: + +- **`jobs`** — повторно используются существующие колонки для пути провала: + `status` (`queued`/`running`/`done`/`failed`), `error`, `attempts`, `max_attempts`, + `transient_attempts`, `available_at`, `run_id`. Пустой/невалидный результат идёт тем же + путём, что и обычный permanent/transient провал (`mark_job` / `mark_job_transient`). +- **`agent_runs`** — `exit_code` пишется без искажения (реальный код выхода процесса). + Решение done/fail принимается по отдельному in-memory флагу `result_ok` в `_monitor_agent`, + а не по колонке. + +## Состояние данных +- Никаких бэкофиллов / data-migration. +- Auth-проверка читает **файл** `.credentials.json` (вне БД), результат кешируется in-memory + (`preflight._cache`), не персистится. diff --git a/docs/work-items/ORCH-044/10-tech-risks.md b/docs/work-items/ORCH-044/10-tech-risks.md new file mode 100644 index 0000000..8e1521a --- /dev/null +++ b/docs/work-items/ORCH-044/10-tech-risks.md @@ -0,0 +1,20 @@ +# 10 — Технические риски + +**Work Item:** ORCH-044 +**Основано на:** ADR-001 + +| ID | Риск | Вероятн. | Влияние | Митигация | +|----|------|----------|---------|-----------| +| R-1 | **Ложноположительный auth-fail.** Неверно резолвленный путь к `.credentials.json` (иной HOME/маунт) ⇒ preflight всегда FAIL ⇒ **не клеймится ни одна job всех проектов** (общая очередь, self-hosting). | Средняя | **Высокое** | Единый источник HOME (`AgentLauncher.AGENT_HOME`, зеркально `_claude_bin()`); тумблер `ORCH_PREFLIGHT_CHECK_AUTH=false`; **обязательная проверка на staging** (реальный путь == резолвленный) перед прод-деплоем; информативный reason в `/queue` + warning-лог. | +| R-2 | **Fail-safe-провал на легитимном пустом выводе.** Агент легитимно завершился `exit 0` с непустым логом, но `_validate_result` ошибочно счёл результат невалидным ⇒ ложный `failed`/requeue (регрессия AC-13). | Низкая | Среднее | Контракт извлечения JSON — тот же, что у работающего usage-учёта (`_extract_last_json_object`); регресс-тест TC-15 (валидный лог ⇒ `done`); валидатор не трогает успешный путь, кроме булева флага. | +| R-3 | **`expiresAt` без сетевой валидации.** Реально отозванный, но ещё не истёкший по времени токен пройдёт проактивную проверку (a). | Средняя | Среднее | Защитная сетка постфактум (b): маркер `Not logged in` в логе ⇒ `error` + `preflight.reset_cache()` ⇒ следующий тик переоценивает auth; полная сетевая валидация — вне scope (BR-1). | +| R-4 | **`expiresAt` отсутствует/нечисловой** в файле (иная версия CLI / иной формат) ⇒ проверка трактует как OK и пропускает. | Низкая | Низкое | Осознанный компромисс против ложных срабатываний (см. ADR §P1.5); отсутствие токена/accessToken по-прежнему ⇒ FAIL; постфактум-маркер ловит реальный «не залогинен». | +| R-5 | **Часовой рассинхрон** контейнер vs токен ⇒ валидный токен сочтён истёкшим. | Низкая | Среднее | `ORCH_AUTH_EXPIRY_SKEW_SECONDS` (default 0) для запаса; контейнеры на одном хосте (mva154) — рассинхрон маловероятен. | +| R-6 | **Транзиентный auth (битый JSON в момент записи re-login).** Чтение файла во время атомарной перезаписи ⇒ временный FAIL. | Низкая | Низкое | Кеш TTL сглаживает; следующий тик перечитает; fail-safe в сторону «подождать» (job остаётся `queued`, не теряется). | +| R-7 | **Конфликт test-plan с коррекцией scope.** `04-test-plan.yaml` TC-09/TC-10/TC-11 проверяют `--effort` (variant B: «`--effort` не формируется»), но владелец **исключил** effort из ORCH-044 и оставил дефолты `agent_effort_*` = `high`. При дефолтной тест-конфигурации `_spawn` сформирует `--effort high` ⇒ TC-09 (ожидающий отсутствие флага) **упадёт**. | **Высокая** | Среднее | Developer/Tester: **адаптировать TC-09/10/11** под «effort не трогаем» (assert успешной сборки cmd без требования удаления флага, либо пометить как deferred→ORCH-50). Артефакт `04-test-plan.yaml` — чужой этап (правило №3), архитектор его НЕ редактирует, только фиксирует расхождение здесь. AC-7/AC-8/AC-9 не применяются (см. `03-acceptance-criteria.md` §P2). | +| R-8 | **Постфактум auth-сброс кеша зацикливает.** Повторные auth-провалы ⇒ повторные `reset_cache()`. | Низкая | Низкое | `reset_cache()` лишь форсирует один пересчёт; следующий `check()` снова закеширует на TTL; цикла «горячего» чтения нет; job не клеймится при FAIL. | + +## Сводно +Доминирующий риск — **R-1** (блок очереди ложным auth-fail при неверном пути) и +организационный **R-7** (test-plan vs scope). Оба закрываются: R-1 — staging-проверкой + +тумблером, R-7 — правкой effort-тестов разработчиком/тестером согласно коррекции владельца. diff --git a/docs/work-items/ORCH-044/12-review.md b/docs/work-items/ORCH-044/12-review.md new file mode 100644 index 0000000..6d05a3e --- /dev/null +++ b/docs/work-items/ORCH-044/12-review.md @@ -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()` — чтение `/.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. diff --git a/docs/work-items/ORCH-044/13-test-report.md b/docs/work-items/ORCH-044/13-test-report.md new file mode 100644 index 0000000..294d645 --- /dev/null +++ b/docs/work-items/ORCH-044/13-test-report.md @@ -0,0 +1,84 @@ +--- +type: test-report +work_item_id: ORCH-044 +result: PASS +--- + +# Test Report — ORCH-044 + +Надёжность запуска агента: preflight auth (P1) + пустой лог = провал (P3). +**P2 (`--effort`) исключён из scope владельцем** (06.06) — вынесен в ORCH-50; +AC-7/AC-8/AC-9 и TC-09/TC-11 (effort) в этой задаче **не применяются (N/A)**. + +## Окружение +- Python: 3.12.13 +- pytest: 8.3.3 +- Branch: feature/ORCH-044-preflight-auth-effort +- Дата: 2026-06-06T08:39Z +- Прод-инстанс (8500): не трогался; smoke — read-only GET. + +## Результаты — Quality Gate тесты (04-test-plan.yaml) + +| TC ID | Описание | Тест(ы) | Результат | +|-------|----------|---------|-----------| +| TC-01 | Нет `.credentials.json` ⇒ FAIL про auth | `test_missing_credentials_fails` | PASS | +| TC-02 | Протухший OAuth `expiresAt` ⇒ FAIL | `test_expired_token_fails` | PASS | +| TC-03 | Валидный логин ⇒ OK без регрессии | `test_valid_login_ok` | PASS | +| TC-04 | Битый JSON ⇒ FAIL без исключения | `test_broken_json_fails_without_raising` | PASS | +| TC-05 | Token-free: нет сетевого вызова | `test_auth_check_makes_no_network_call` | PASS | +| TC-06 | Кеширование auth в пределах TTL | `test_auth_result_cached_within_ttl` | PASS | +| TC-07 | Путь credentials от HOME агента (/home/slin) | `test_credentials_path_follows_agent_home` | PASS | +| TC-08 | Worker не клеймит job при auth-fail | `test_worker_does_not_claim_when_auth_fails` | PASS | +| TC-09 | (effort) cmd без `--effort` | `test_effort_flag.py` | N/A — scope исключён владельцем (ORCH-50) | +| TC-10 | `resolve_agent_effort` согласован | `test_resolve_agent_effort.py` (11 тестов) | PASS — effort не тронут, тесты зелёные | +| TC-11 | (effort) дефолтный путь даёт непустой JSON | `test_effort_flag.py` | N/A — scope исключён владельцем (ORCH-50) | +| TC-12 | Пустой лог + exit0 ⇒ failed + алерт | `test_empty_log_exit0_terminal_failed_alerts` | PASS | +| TC-13 | Лог без result-JSON ⇒ провал | `test_garbage_log_exit0_not_done` | PASS | +| TC-14 | Провал ⇒ нет advance/успешного коммента | `test_empty_result_suppresses_advance_and_comment` | PASS | +| TC-15 | Валидный JSON ⇒ done без регрессии | `test_valid_result_done`, `test_success_advances_and_comments` | PASS | +| TC-16 | Никогда не вечный `running` | `test_never_running_after_empty_result` | PASS | +| TC-17 | Классификация permanent/transient | `test_empty_result_defaults_permanent`, `..._with_transient_marker_goes_transient` | PASS | +| TC-18 | Регресс preflight (bin/version) | `test_resilience.py::TestPreflight` | PASS | +| TC-19 | Полный `pytest tests/` зелёный | вся сюита | PASS (504 passed) | + +Дополнительно покрыто (вне нумерации плана): постфактум auth-маркер +(`test_is_auth_failure_text_*`, `TestAuthMarkerHandling`), тумблер +`ORCH_PREFLIGHT_CHECK_AUTH` (`test_auth_toggle_off_skips_check`), явный путь +credentials (`test_explicit_credentials_path_wins`). + +## Сопоставление с критериями приёмки +- **AC-1…AC-6** (preflight auth): PASS — TC-01…TC-08. +- **AC-7/AC-8/AC-9** (effort): N/A — исключены владельцем (см. 02-trz.md, 03-acceptance-criteria.md). +- **AC-10…AC-14** (пустой результат ⇒ провал): PASS — TC-12…TC-16. +- **AC-15** (документация в том же PR): PASS — подтверждено review (APPROVED): README/internals/INFRA/CHANGELOG/ADR обновлены. +- **AC-16** (тесты зелёные): PASS — 504 passed. +- **AC-17** (self-hosting): PASS — изменения в слое preflight/launch; прод-контейнер не рестартовался; smoke 8500 read-only. + +## Smoke test API (8500, read-only GET) +| Endpoint | Код | Замечание | +|----------|-----|-----------| +| GET /health | 200 | `{"status":"ok","service":"orchestrator"}` | +| GET /status | 200 | активна задача ORCH-044 (stage=testing) | +| GET /queue | 200 | counts ok (failed=0), `preflight_ok=true`, breaker=closed | + +> curl в окружении отсутствует — smoke выполнен через `urllib` (эквивалентные GET). + +## Вывод pytest +``` +======================= 504 passed, 1 warning in 10.82s ======================== +``` +Модули плана (детально): +``` +tests/test_preflight_auth.py ......... 18 passed +tests/test_resolve_agent_effort.py ... 11 passed +tests/test_empty_log_failure.py ...... 18 passed +tests/test_resilience.py ............. 31 passed +(итого по модулям плана: 78 passed) +``` +Warning: `PydanticDeprecatedSince20` в `src/config.py:4` — предсуществующий, +вне scope ORCH-044 (зафиксировано в review как P2/опционально). + +## Итог +**PASS** — все применимые тесты плана зелёные, существующая сюита не сломана, +smoke API исправен. TC-09/TC-11 (effort) корректно N/A: P2 исключён владельцем +и вынесен в ORCH-50. Задача готова к стадии **deploy-staging**. diff --git a/docs/work-items/ORCH-044/14-deploy-log.md b/docs/work-items/ORCH-044/14-deploy-log.md new file mode 100644 index 0000000..7922bc3 --- /dev/null +++ b/docs/work-items/ORCH-044/14-deploy-log.md @@ -0,0 +1,90 @@ +--- +deploy_status: SUCCESS +timestamp: 2026-06-06T08:44:04Z +work_item: ORCH-044 +branch: feature/ORCH-044-preflight-auth-effort +commit: 08ace892bbf1809a65c1dc504459d052bfd71f79 +target_service: orchestrator +target_port: 8500 +deploy_mode: artifact-only +staging_gate: SUCCESS +prod_container_restarted: false +rebuild_required: true +--- + +# Deploy Log — ORCH-044 + +## Verdict + +**`deploy_status: SUCCESS`** — артефактный (artifact-only) деплой-вердикт. + +Реальный `git pull` + `docker compose ... --build` + рестарт прод-контейнера +`orchestrator` (8500) в рамках этой стадии **НЕ выполняется**. Он делегирован +хуку `scripts/orchestrator-deploy-hook.sh` (ORCH-36), который запускается +Владельцем **после** мерджа ветки `feature/ORCH-044-preflight-auth-effort` в +`main`. Guardrail: агент никогда не перезапускает общий прод-инстанс внутри +ORCH-задачи (CLAUDE.md / INFRA.md §Self-hosting). + +## Pre-conditions (все ✓) + +| Артефакт | Поле | Значение | +|----------|------|----------| +| `12-review.md` | `verdict` | `APPROVED` | +| `13-test-report.md` | `result` | `PASS` | +| `15-staging-log.md` (origin/main) | `staging_status` | `SUCCESS` (10/10 staging-checks, прогон внутри `orchestrator-staging` :8501) | +| `04-test-plan.yaml` | — | покрывает AC (P2/`--effort` исключён владельцем → ORCH-50, N/A) | +| ADR | `06-adr/ADR-001-preflight-auth-and-empty-result-failure.md` | заведён | +| `CHANGELOG.md` | — | обновлён | + +Стадия `deploy` достижима только потому, что условный staging-гейт +(`check_staging_status`, реальный для self-hosting repo=orchestrator) — зелёный. + +## Change scope — почему нужен rebuild+restart (но не сейчас) + +В отличие от чисто bind-mount изменений (ср. ORCH-048), ORCH-044 меняет +**рантайм-код `src/`**, который копируется в образ (`/app/src`) и исполняется +прод-процессом: + +| Файл | Тип | Как доезжает до прода | +|------|-----|------------------------| +| `src/preflight.py` | runtime (в образе) | требует **rebuild + restart** контейнера | +| `src/agents/launcher.py` | runtime (в образе) | требует **rebuild + restart** контейнера | +| `src/config.py` | runtime (в образе) | требует **rebuild + restart** контейнера | +| `docs/**`, `CHANGELOG.md` | docs | мерж в `main` | +| `tests/**` | тесты, не деплоятся | n/a | + +`rebuild_required: true`. Чтобы новый token-free auth-гейт preflight и +«пустой лог ⇒ провал» вступили в силу на проде, прод-инстанс `orchestrator` +(8500) должен быть пересобран и перезапущен. **Это делает Владелец через +деплой-хук после мерджа**, не данный агент. + +## Self-hosting policy + +> ORCH-044 правит слой запуска агента (preflight/launcher/config) того самого +> инструмента, который СЕЙЧАС обслуживает все прод-проекты (orchestrator + +> enduro-trails) из одного инстанса `orchestrator:8500` с общей БД и общей +> очередью. + +Поэтому в рамках этой стадии: +- **Прод-контейнер `orchestrator` (8500) НЕ трогался** — ни рестарта, ни + пересборки (групповой риск для всех проектов). +- **Деплой-хук** `scripts/orchestrator-deploy-hook.sh` (реальный docker/SSH) + **не запускался** этим агентом (не было явной инструкции Owner; зарезервирован + за ним, ORCH-36). У хука есть health-цикл (10×6с) + авто-rollback — + страховка на момент боевого rebuild+restart. +- **Страховка пройдена:** staging (8501, изолированная БД/реестр) — зелёный + перед прод-деплоем (ORCH-35). + +## Deploy action + +- **Prod rebuild/restart:** требуется (`src/` изменён), **не выполнен** этим + агентом (guardrail self-hosting). Выполняется Владельцем через деплой-хук + после мерджа в `main`. +- **Эффективный rollout:** мерж ветки в `main` → Owner запускает + `scripts/orchestrator-deploy-hook.sh` (прод-режим: `TARGET_SERVICE=orchestrator + TARGET_PORT=8500 COMPOSE_PROFILE=""`) с health-check + авто-rollback. + +## Verdict + +`deploy_status: SUCCESS` — все гейты зелёные, артефакт-вердикт зафиксирован, +боевой rebuild+restart делегирован Owner-хуку. Прод-инстанс не затронут. diff --git a/src/agents/launcher.py b/src/agents/launcher.py index 43d8019..816601a 100644 --- a/src/agents/launcher.py +++ b/src/agents/launcher.py @@ -185,6 +185,10 @@ class AgentLauncher: } CLAUDE_BIN = "/opt/claude-code/bin/claude.exe" + # ORCH-044 (P1): HOME the claude subprocess actually runs under. preflight + # resolves the OAuth credentials path from this (NOT the orchestrator process + # HOME), so keep this single source of truth in sync with the spawn env below. + AGENT_HOME = "/home/slin" # ORCH-7 (M-2): timeout is now configurable. AGENT_TIMEOUT stays as a # backward-compatible alias for the default; the actual value (and per-agent # overrides) live in settings and are resolved via _resolve_timeout(). @@ -323,7 +327,7 @@ class AgentLauncher: stderr=subprocess.STDOUT, env={ **os.environ, - "HOME": "/home/slin", + "HOME": self.AGENT_HOME, "GIT_AUTHOR_NAME": "claude-bot", "GIT_AUTHOR_EMAIL": "claude-bot@mva154.local", "GIT_COMMITTER_NAME": "claude-bot", @@ -492,6 +496,21 @@ class AgentLauncher: notify_agent_finished(run_id, agent, exit_code, task_id=_task_id, duration_s=_duration_s) + # ORCH-044 (P3): a clean exit_code==0 is NOT enough — claude can die fast + # (logged out, killed flag) leaving an empty / JSON-less log while still + # exiting 0. Validate the result; only (exit 0 AND result_ok) is success. + # The real exit_code is still recorded above without distortion; this flag + # drives the done/fail decision (ADR-001 §P3 / A4). + result_ok, result_reason = (True, "ok") + if exit_code == 0: + result_ok, result_reason = self._validate_result(output_path) + if not result_ok: + logger.warning( + f"Agent run_id={run_id} ({agent}) exited 0 but result invalid: " + f"{result_reason}" + ) + success = (exit_code == 0 and result_ok) + # Feature 4: parse token usage / cost from the (json) run log and record # it on the agent_runs row. Never fatal — a garbled/missing JSON records # NULLs and logs a warning so a broken run can't crash the monitor. @@ -510,7 +529,7 @@ class AgentLauncher: try: git_env = { **os.environ, - "HOME": "/home/slin", + "HOME": self.AGENT_HOME, "GIT_AUTHOR_NAME": "claude-bot", "GIT_AUTHOR_EMAIL": "claude-bot@mva154.local", "GIT_COMMITTER_NAME": "claude-bot", @@ -593,11 +612,34 @@ class AgentLauncher: from ..notifications import send_telegram send_telegram(f"\u26a0\ufe0f {_wid}: Agent {agent} failed (exit_code={exit_code}). Check logs: /app/data/runs/{run_id}.log") + # ORCH-044 (P3): exit 0 with an empty/invalid result is a failure, not a + # success — alert (like other failures) and DO NOT post a success comment + # or advance the stage. The job-queue finalize below routes it to + # failed/retry. (AC-10/11/12.) + if exit_code == 0 and not success: + try: + conn = get_db() + task_row = conn.execute( + "SELECT work_item_id FROM tasks WHERE repo=? AND branch=?", + (repo, branch), + ).fetchone() + conn.close() + _wid = task_row[0] if task_row else None + from ..notifications import send_telegram + send_telegram( + f"⚠️ {_wid or repo}: Agent {agent} exited 0 but produced " + f"an empty/invalid result ({result_reason}). " + f"Logs: /app/data/runs/{run_id}.log" + ) + except Exception as e: + logger.warning(f"run_id={run_id}: empty-result alert failed: {e}") + # Feature 4 + ORCH-016: post the unified per-agent status comment under # that agent's bot, threading the wall-clock duration we just measured # straight through (ADR-001 §6: explicit param wins over DB fallback). # The deployer finishing the task also posts the per-task usage summary. - if exit_code == 0: + # ORCH-044 (P3): only on real success (exit 0 AND valid result). + if success: try: self._post_usage_comments( run_id, agent, repo, branch, _usage, duration_s=_duration_s @@ -605,14 +647,81 @@ class AgentLauncher: except Exception as e: logger.warning(f"run_id={run_id}: usage comment failed: {e}") - # Auto-advance stage if agent finished successfully and QG passes - if exit_code == 0: + # Auto-advance stage if agent finished successfully and QG passes. + # ORCH-044 (P3): suppressed when the result was empty/invalid. + if success: self._try_advance_stage(run_id, agent, repo, branch) # ORCH-1: drive the job-queue status for queue-launched jobs only. # (Legacy direct launch() has job_id=None and is unaffected.) + # ORCH-044 (P3): result_ok lets _finalize_job treat an empty-result exit 0 + # as a failure rather than 'done'. if job_id is not None: - self._finalize_job(job_id, agent, run_id, exit_code, output_path=output_path) + self._finalize_job( + job_id, agent, run_id, exit_code, + output_path=output_path, result_ok=result_ok, + ) + + @staticmethod + def _validate_result(output_path) -> tuple[bool, str]: + """ORCH-044 (P3): is the run log a real result, or an empty/JSON-less death? + + Returns (ok, reason). A run counts as a valid result only when the log + exists, is non-empty (not just whitespace), AND carries a parseable + trailing result-JSON object — the same contract usage accounting uses + (usage._extract_last_json_object). claude --output-format json always + emits exactly such an object on a real run, so its absence means the agent + died before producing anything. + + Never raises: any error is treated as an invalid result (fail-safe toward + failing the job rather than silently passing — TR-3.5). + """ + try: + if not output_path: + return False, "no output path" + if not os.path.exists(output_path): + return False, "run log missing" + if os.path.getsize(output_path) == 0: + return False, "empty run log (0 bytes)" + with open(output_path, "r", encoding="utf-8", errors="replace") as f: + text = f.read() + if not text.strip(): + return False, "empty run log (whitespace only)" + from ..usage import _extract_last_json_object + if _extract_last_json_object(text) is None: + return False, "no result JSON in run log" + return True, "result ok" + except Exception as e: # pragma: no cover - defensive fail-safe + return False, f"result validation error: {e}" + + def _handle_auth_marker(self, log_path) -> bool: + """ORCH-044 (P1b): post-factum auth-failure detection (defensive net). + + If an agent died because the session was logged out / expired between + preflight and spawn, reset the preflight cache so the NEXT worker tick + re-evaluates auth proactively (fast re-login pickup, or continued gating + if still broken). Auth failure is deliberately NOT treated as transient + and does NOT crank the circuit breaker — preflight is the right gate here. + Returns True if an auth marker was found. Never raises. + """ + try: + from .. import preflight + with open(log_path, "rb") as f: + try: + f.seek(-16384, 2) + except OSError: + f.seek(0) + text = f.read().decode("utf-8", errors="replace") + if preflight.is_auth_failure_text(text): + logger.warning( + f"Auth-failure marker in {log_path}; resetting preflight cache " + f"so the next tick re-checks auth" + ) + preflight.reset_cache() + return True + except Exception: + pass + return False def _backoff_seconds(self, transient_attempts: int, retry_after: int = None) -> int: """Exponential backoff for transient failures, honouring Retry-After. @@ -627,17 +736,21 @@ class AgentLauncher: backoff = max(backoff, min(retry_after, cap)) return int(backoff) - def _finalize_job(self, job_id: int, agent: str, run_id: int, exit_code, output_path=None): + def _finalize_job(self, job_id: int, agent: str, run_id: int, exit_code, + output_path=None, result_ok: bool = True): """ORCH-1: update the jobs row after the agent process finished. - exit_code == 0 -> done (and resets the breaker streak via on_outcome). - exit_code != 0 -> classify the failure from the run log tail (token-free): + success = (exit_code == 0 AND result_ok) -> done (resets the breaker + streak via on_outcome). ORCH-044 (P3): result_ok==False means + exit 0 but the run log was empty / had no result-JSON, so it is + routed through the failure path below, NOT marked done. + otherwise -> classify the failure from the run log tail (token-free): - TRANSIENT (429/overload/network): backoff-requeue with available_at in the future + a SEPARATE transient_attempts budget (settings.transient_max_attempts), honouring Retry-After. Reported to the breaker so it opens after N consecutive transient failures. - - PERMANENT (code fault): ordinary attempts < max_attempts requeue, - otherwise 'failed' + Telegram. + - PERMANENT (code fault, incl. the empty-result case): ordinary + attempts < max_attempts requeue, otherwise 'failed' + Telegram. """ from ..db import get_job, mark_job from ..error_classifier import classify_log_file @@ -645,34 +758,55 @@ class AgentLauncher: job = get_job(job_id) if not job: return - if exit_code == 0: + if exit_code == 0 and result_ok: mark_job(job_id, "done", run_id=run_id) logger.info(f"Job {job_id} ({agent}) done (run_id={run_id})") self._record_outcome(transient=False, recovered=True) return + log_path = output_path or f"/app/data/runs/{run_id}.log" + + # ORCH-044 (P1b): if the failure was an auth death, invalidate the + # preflight cache so the next tick re-gates on auth proactively. + self._handle_auth_marker(log_path) + + # ORCH-044 (P3): informative error for the empty/invalid-result case + # (exit 0 but no usable result). Defaults to permanent (it is not a + # 429/overload) unless the log carries a transient marker (TR-3.3). + empty_result = (exit_code == 0 and not result_ok) + override_err = ( + f"empty run log / no result JSON (run_id={run_id})" + if empty_result else None + ) + # Classify the failure from the agent log tail (no token cost). kind, retry_after = "permanent", None - log_path = output_path or f"/app/data/runs/{run_id}.log" try: kind, retry_after = classify_log_file(log_path) except Exception: pass if kind == "transient": - self._finalize_transient(job_id, agent, run_id, exit_code, job, retry_after) + self._finalize_transient(job_id, agent, run_id, exit_code, job, + retry_after, error=override_err) else: - self._finalize_permanent(job_id, agent, run_id, exit_code, job) + self._finalize_permanent(job_id, agent, run_id, exit_code, job, + error=override_err) except Exception as e: logger.error(f"Job {job_id}: _finalize_job error: {e}") - def _finalize_transient(self, job_id, agent, run_id, exit_code, job, retry_after): - """Transient (429/overload/net) failure -> backoff requeue or fail when budget out.""" + def _finalize_transient(self, job_id, agent, run_id, exit_code, job, retry_after, + error: str | None = None): + """Transient (429/overload/net) failure -> backoff requeue or fail when budget out. + + ORCH-044 (P3): `error`, when provided, overrides the default transient + message (used for the empty-result case so the reason is informative). + """ from ..db import mark_job, mark_job_transient tattempts = job.get("transient_attempts", 0) tmax = settings.transient_max_attempts - err = (f"transient (429/overload) agent {agent} exit={exit_code} " - f"(run_id={run_id}); retry_after={retry_after}") + err = error or (f"transient (429/overload) agent {agent} exit={exit_code} " + f"(run_id={run_id}); retry_after={retry_after}") self._record_outcome(transient=True, recovered=False) if tattempts < tmax: backoff = self._backoff_seconds(tattempts + 1, retry_after) @@ -689,12 +823,17 @@ class AgentLauncher: self._notify_failed(job_id, agent, job, run_id, f"transient (rate-limit) after {tattempts} attempts") - def _finalize_permanent(self, job_id, agent, run_id, exit_code, job): - """Permanent (code-fault) failure -> normal attempts normal attempts consecutive transient failures that OPEN the breaker. # breaker_pause_seconds -> how long the breaker stays open before half-open. preflight_cache_ttl: int = 45 + # ORCH-044 (P1): token-free preflight auth gate. After `claude --version` + # succeeds, preflight also checks that claude is logged in by reading the + # local OAuth credentials file (no network / no prompt-ping — BR-1). + # preflight_check_auth -> master toggle (env ORCH_PREFLIGHT_CHECK_AUTH). + # Emergency off-switch if the check ever + # false-positives and wedges the shared queue. + # claude_credentials_path -> explicit path to .credentials.json + # (env ORCH_CLAUDE_CREDENTIALS_PATH). Empty -> + # /.claude/.credentials.json, where + # AGENT_HOME is the HOME the launcher really + # spawns claude under (/home/slin), NOT the + # orchestrator process env. + # auth_expiry_skew_seconds -> clock-drift slack when comparing + # claudeAiOauth.expiresAt (env + # ORCH_AUTH_EXPIRY_SKEW_SECONDS); a token within + # this many seconds of now is treated as expired. + preflight_check_auth: bool = True + claude_credentials_path: str = "" + auth_expiry_skew_seconds: int = 0 backoff_base_seconds: int = 10 backoff_max_seconds: int = 600 transient_max_attempts: int = 5 diff --git a/src/preflight.py b/src/preflight.py index 316fdce..f9d0f02 100644 --- a/src/preflight.py +++ b/src/preflight.py @@ -5,14 +5,25 @@ are reachable WITHOUT spending any tokens. We only do local/cheap checks: 1. os.path.exists(CLAUDE_BIN) -- instant 2. `claude --version` (timeout 5s) -- spawns CLI, does NOT call the API + 3. auth check (ORCH-044, P1) -- read the local OAuth credentials file The result is cached for `preflight_cache_ttl` seconds so we do not re-run -`claude --version` on every worker tick. +`claude --version` (or re-read the credentials file) on every worker tick. 🚫 We deliberately do NOT do a prompt ping (ping->pong) — that would burn the rate limit and add latency. Preflight is local-only. + +ORCH-044 (P1): `claude --version` answers successfully even when claude is NOT +logged in (the version is local information), so version-only preflight was blind +to auth. We add a token-free auth gate: read /.claude/.credentials.json +and validate the OAuth token (presence + expiry). Combined with a post-factum +`Not logged in` marker detection (is_auth_failure_text), this stops a logged-out +instance from claiming jobs and silently dying with an empty run log. No network +call is ever made here. """ import os +import re +import json import time import logging import subprocess @@ -23,6 +34,15 @@ logger = logging.getLogger("orchestrator.preflight") _VERSION_TIMEOUT = 5 +# ORCH-044 (P1b): post-factum auth-failure markers. If an agent started under a +# session that died/expired between preflight and spawn, these substrings in the +# run log identify the auth failure so the launcher can invalidate the preflight +# cache (forcing the next tick to re-evaluate auth proactively). +_AUTH_FAIL_RE = re.compile( + r"not logged in|please run\s*/login|invalid api key|unauthorized|\b401\b", + re.IGNORECASE, +) + class _PreflightCache: def __init__(self): @@ -74,11 +94,120 @@ def _run_version(bin_path: str) -> tuple[bool, str]: return False, f"--version error: {e}" +def _agent_home() -> str: + """Resolve the HOME the launcher actually spawns claude under (ORCH-044, TR-1.3). + + The auth credentials live under the *agent's* HOME (/home/slin), which the + launcher injects into the claude subprocess env — NOT the orchestrator + process HOME. We mirror _claude_bin()'s "follow the genuinely executed path" + approach by reading AgentLauncher.AGENT_HOME. Falls back to the known default + if the launcher cannot be imported (e.g. isolated unit test). + """ + try: + from .agents.launcher import AgentLauncher + home = getattr(AgentLauncher, "AGENT_HOME", None) + if home: + return home + except Exception: + pass + return "/home/slin" + + +def _credentials_path() -> str: + """Path to claude's OAuth credentials file (ORCH-044, P1). + + settings.claude_credentials_path wins when set; otherwise + /.claude/.credentials.json. + """ + explicit = (getattr(settings, "claude_credentials_path", "") or "").strip() + if explicit: + return explicit + return os.path.join(_agent_home(), ".claude", ".credentials.json") + + +def _iso(epoch_ms) -> str: + """Best-effort epoch-ms -> ISO-8601 UTC string (for human-readable reasons).""" + try: + from datetime import datetime, timezone + return datetime.fromtimestamp(int(epoch_ms) / 1000, tz=timezone.utc).isoformat() + except Exception: + return str(epoch_ms) + + +def is_auth_failure_text(text: str) -> bool: + """ORCH-044 (P1b): True if `text` contains a claude auth-failure marker. + + Used post-factum on a run log so the launcher can tell an auth death apart + from a generic failure and reset the preflight cache. Never raises. + """ + if not text: + return False + try: + return bool(_AUTH_FAIL_RE.search(text)) + except Exception: + return False + + +def _check_auth() -> tuple[bool, str]: + """ORCH-044 (P1a): token-free local auth gate. Never raises. + + Steps (ADR-001 §P1): + 1. credentials file missing / unreadable / invalid JSON -> not ok. + 2. no claudeAiOauth block / accessToken -> not ok. + 3. claudeAiOauth.expiresAt (epoch ms) <= now + skew -> expired -> not ok. + 4. accessToken present but expiresAt absent/unparsable -> OK (cannot prove + expiry; we do not manufacture false positives that would wedge the shared + queue — see ADR Risks R-1). + + Fail-safe: any unexpected error returns (False, ...) so a logged-out / broken + state never claims a job (BR-2 / TR-3.5). This reads only a local file — no + network call, no token spend (BR-1 / AC-5). + """ + try: + path = _credentials_path() + if not os.path.exists(path): + return False, f"claude not logged in: credentials missing ({path})" + try: + with open(path, "r", encoding="utf-8") as f: + data = json.load(f) + except (OSError, ValueError) as e: + return False, f"claude not logged in: credentials unreadable ({e})" + + oauth = data.get("claudeAiOauth") if isinstance(data, dict) else None + if not isinstance(oauth, dict) or not oauth.get("accessToken"): + return False, "claude not logged in: no oauth token" + + expires = oauth.get("expiresAt") + if expires is None: + return True, "auth ok (no expiry recorded)" + try: + expires_ms = int(expires) + except (TypeError, ValueError): + return True, "auth ok (unparsable expiry)" + + skew_ms = int(getattr(settings, "auth_expiry_skew_seconds", 0) or 0) * 1000 + now_ms = int(time.time() * 1000) + if expires_ms <= now_ms + skew_ms: + return False, f"OAuth token expired at {_iso(expires_ms)}" + return True, "auth ok" + except Exception as e: # pragma: no cover - defensive fail-safe + return False, f"auth check error: {e}" + + def _compute() -> tuple[bool, str]: bin_path = _claude_bin() if not os.path.exists(bin_path): return False, f"CLAUDE_BIN not found: {bin_path}" - return _run_version(bin_path) + ok, reason = _run_version(bin_path) + if not ok: + return ok, reason + # ORCH-044 (P1): version is local info and answers even when logged out, so + # gate on a token-free auth check too. Toggleable for emergencies. + if getattr(settings, "preflight_check_auth", True): + auth_ok, auth_reason = _check_auth() + if not auth_ok: + return False, auth_reason + return True, reason def check(force: bool = False) -> tuple[bool, str]: diff --git a/tests/test_empty_log_failure.py b/tests/test_empty_log_failure.py new file mode 100644 index 0000000..4d014e2 --- /dev/null +++ b/tests/test_empty_log_failure.py @@ -0,0 +1,298 @@ +"""ORCH-044 (P3): empty run log / no result-JSON at exit 0 == failure. + +claude can exit 0 yet leave an empty (or JSON-less) run log — e.g. it died fast +because the session was logged out, or a flag silenced stdout. Before ORCH-044 +that looked identical to success: job -> done, stage auto-advanced. Now the +launcher validates the result; only (exit 0 AND valid result-JSON) is a success. + +No real claude/Popen is spawned. The git/usage/notify side effects of +_monitor_agent are stubbed; DB is a fresh per-test sqlite. +""" +import os +import tempfile + +import pytest + +_test_db = os.path.join(tempfile.gettempdir(), "test_orchestrator_empty_log.db") +os.environ["ORCH_DB_PATH"] = _test_db +os.environ["ORCH_REPOS_DIR"] = tempfile.gettempdir() +os.environ["ORCH_GITEA_TOKEN"] = "test-token" +os.environ["ORCH_PLANE_API_TOKEN"] = "test-token" + +import src.db as db +from src.db import init_db, enqueue_job, claim_next_job, get_job +from src import preflight +from src.agents.launcher import AgentLauncher + + +VALID_RESULT_LOG = ( + "some preamble text from the agent run...\n" + '{"type":"result","subtype":"success","usage":' + '{"input_tokens":120,"output_tokens":45},"total_cost_usd":0.12}\n' +) + + +@pytest.fixture(autouse=True) +def fresh_db(tmp_path, monkeypatch): + monkeypatch.setattr(db.settings, "db_path", str(tmp_path / "res.db")) + init_db() + preflight.reset_cache() + yield + + +# =========================================================================== +# _validate_result — the result-JSON contract (TR-3.1) +# =========================================================================== +class TestValidateResult: + def test_missing_path(self): + ok, reason = AgentLauncher._validate_result(None) + assert ok is False + + def test_missing_file(self, tmp_path): + ok, reason = AgentLauncher._validate_result(str(tmp_path / "nope.log")) + assert ok is False + assert "missing" in reason.lower() + + def test_empty_file(self, tmp_path): + p = tmp_path / "empty.log" + p.write_text("") + ok, reason = AgentLauncher._validate_result(str(p)) + assert ok is False + assert "empty" in reason.lower() + + def test_whitespace_only(self, tmp_path): + p = tmp_path / "ws.log" + p.write_text(" \n\t\n") + ok, _ = AgentLauncher._validate_result(str(p)) + assert ok is False + + def test_no_json(self, tmp_path): + p = tmp_path / "garbage.log" + p.write_text("this is not json at all, just noise\n") + ok, reason = AgentLauncher._validate_result(str(p)) + assert ok is False + assert "json" in reason.lower() + + def test_valid_result_json(self, tmp_path): + p = tmp_path / "good.log" + p.write_text(VALID_RESULT_LOG) + ok, _ = AgentLauncher._validate_result(str(p)) + assert ok is True + + +# =========================================================================== +# _finalize_job — job state under result_ok (TC-12/13/15/16/17) +# =========================================================================== +class TestFinalizeJobResultOk: + def _spy_telegram(self, monkeypatch): + sent = [] + monkeypatch.setattr("src.notifications.send_telegram", + lambda *a, **k: sent.append(a[0] if a else "")) + return sent + + # TC-15 / AC-13: valid result -> done (no regression). + def test_valid_result_done(self, tmp_path, monkeypatch): + self._spy_telegram(monkeypatch) + log = tmp_path / "1.log" + log.write_text(VALID_RESULT_LOG) + jid = enqueue_job("developer", "r") + claim_next_job() + AgentLauncher()._finalize_job(jid, "developer", run_id=1, exit_code=0, + output_path=str(log), result_ok=True) + assert get_job(jid)["status"] == "done" + + # TC-12 / AC-10: exit 0 + empty log -> NOT done; terminal failed + alert. + def test_empty_log_exit0_terminal_failed_alerts(self, tmp_path, monkeypatch): + sent = self._spy_telegram(monkeypatch) + log = tmp_path / "2.log" + log.write_text("") # 0 bytes + # max_attempts=1 -> after the claim (attempts=1) the budget is spent -> + # the permanent path goes straight to 'failed' and alerts. + jid = enqueue_job("developer", "r", max_attempts=1) + claim_next_job() + AgentLauncher()._finalize_job(jid, "developer", run_id=2, exit_code=0, + output_path=str(log), result_ok=False) + job = get_job(jid) + assert job["status"] == "failed" + assert job["status"] != "done" + assert "empty run log" in (job["error"] or "") + assert sent, "a Telegram alert must be sent on terminal failure" + + # TC-13 / AC-11: exit 0 + JSON-less log -> failure (here: requeue). + def test_garbage_log_exit0_not_done(self, tmp_path, monkeypatch): + self._spy_telegram(monkeypatch) + log = tmp_path / "3.log" + log.write_text("noise, no json here\n") + jid = enqueue_job("developer", "r", max_attempts=2) + claim_next_job() + AgentLauncher()._finalize_job(jid, "developer", run_id=3, exit_code=0, + output_path=str(log), result_ok=False) + job = get_job(jid) + assert job["status"] != "done" + assert job["status"] == "queued" # retry budget remained + assert "no result JSON" in (job["error"] or "") + + # TC-16 / AC-14: exit 0 + empty log never leaves the job 'running'. + def test_never_running_after_empty_result(self, tmp_path, monkeypatch): + self._spy_telegram(monkeypatch) + log = tmp_path / "4.log" + log.write_text("") + jid = enqueue_job("developer", "r", max_attempts=2) + claim_next_job() + assert get_job(jid)["status"] == "running" # claimed + AgentLauncher()._finalize_job(jid, "developer", run_id=4, exit_code=0, + output_path=str(log), result_ok=False) + assert get_job(jid)["status"] in ("failed", "queued") + + # TC-17 / TR-3.3: empty result defaults to permanent (no backoff, no + # transient budget burn). + def test_empty_result_defaults_permanent(self, tmp_path, monkeypatch): + self._spy_telegram(monkeypatch) + log = tmp_path / "5.log" + log.write_text("") # no transient marker + jid = enqueue_job("developer", "r", max_attempts=2) + claim_next_job() + AgentLauncher()._finalize_job(jid, "developer", run_id=5, exit_code=0, + output_path=str(log), result_ok=False) + job = get_job(jid) + assert job["status"] == "queued" + assert job["transient_attempts"] == 0 # NOT transient + assert job["available_at"] is None # no backoff gate + + # TC-17 / TR-3.3: a transient marker in the log routes to the transient path. + def test_empty_result_with_transient_marker_goes_transient(self, tmp_path, monkeypatch): + self._spy_telegram(monkeypatch) + log = tmp_path / "6.log" + log.write_text("overloaded_error: 429 rate limit. Retry-After: 12\n") + jid = enqueue_job("developer", "r", max_attempts=2) + claim_next_job() + AgentLauncher()._finalize_job(jid, "developer", run_id=6, exit_code=0, + output_path=str(log), result_ok=False) + job = get_job(jid) + assert job["status"] == "queued" + assert job["transient_attempts"] == 1 # transient path taken + assert job["available_at"] is not None # backoff gate set + + +# =========================================================================== +# _monitor_agent — success gating (TC-14/15) + auth-marker reset (P1b) +# =========================================================================== +class _FakeProc: + def __init__(self, exit_code): + self._ec = exit_code + self.pid = 4242 + + def wait(self): + return self._ec + + +def _seed_task_and_run(repo, branch, agent="developer", work_item_id="ORCH-001"): + conn = db.get_db() + conn.execute( + "INSERT INTO tasks (work_item_id, repo, branch, stage) VALUES (?,?,?,?)", + (work_item_id, repo, branch, "development"), + ) + cur = conn.execute( + "INSERT INTO agent_runs (task_id, agent) VALUES ((SELECT id FROM tasks " + "WHERE repo=? AND branch=?), ?)", + (repo, branch, agent), + ) + run_id = cur.lastrowid + conn.commit() + conn.close() + return run_id + + +class TestMonitorAgentGating: + def _patch_monitor_env(self, monkeypatch, tmp_path): + """Stub the heavy side effects of _monitor_agent (git/usage/notify).""" + monkeypatch.setattr("src.agents.launcher.notify_agent_finished", + lambda *a, **k: None) + monkeypatch.setattr("src.agents.launcher.get_worktree_path", + lambda repo, branch: str(tmp_path)) + + class _R: + returncode = 0 + stdout = "" # "no changes to commit" -> skips git add/commit/push + stderr = "" + + monkeypatch.setattr("src.agents.launcher.subprocess.run", + lambda *a, **k: _R()) + + def test_success_advances_and_comments(self, tmp_path, monkeypatch): + self._patch_monitor_env(monkeypatch, tmp_path) + run_id = _seed_task_and_run("r", "feature/x") + log = tmp_path / f"{run_id}.log" + log.write_text(VALID_RESULT_LOG) + + spy = {"post": 0, "advance": 0, "finalize": None, "alert": 0} + monkeypatch.setattr("src.notifications.send_telegram", + lambda *a, **k: spy.__setitem__("alert", spy["alert"] + 1)) + + lr = AgentLauncher() + monkeypatch.setattr(lr, "_post_usage_comments", + lambda *a, **k: spy.__setitem__("post", spy["post"] + 1)) + monkeypatch.setattr(lr, "_try_advance_stage", + lambda *a, **k: spy.__setitem__("advance", spy["advance"] + 1)) + monkeypatch.setattr(lr, "_finalize_job", + lambda *a, **k: spy.__setitem__("finalize", k.get("result_ok"))) + + lr._monitor_agent(_FakeProc(0), run_id, "developer", "r", "feature/x", + output_path=str(log), log_fh=None, job_id=99) + + assert spy["post"] == 1 + assert spy["advance"] == 1 + assert spy["finalize"] is True + assert spy["alert"] == 0 # no empty-result alert on a valid run + + # TC-14 / AC-12: empty result -> no advance, no success comment, alert sent. + def test_empty_result_suppresses_advance_and_comment(self, tmp_path, monkeypatch): + self._patch_monitor_env(monkeypatch, tmp_path) + run_id = _seed_task_and_run("r", "feature/y") + log = tmp_path / f"{run_id}.log" + log.write_text("") # empty -> invalid result + + spy = {"post": 0, "advance": 0, "finalize": None, "alert": 0} + monkeypatch.setattr("src.notifications.send_telegram", + lambda *a, **k: spy.__setitem__("alert", spy["alert"] + 1)) + + lr = AgentLauncher() + monkeypatch.setattr(lr, "_post_usage_comments", + lambda *a, **k: spy.__setitem__("post", spy["post"] + 1)) + monkeypatch.setattr(lr, "_try_advance_stage", + lambda *a, **k: spy.__setitem__("advance", spy["advance"] + 1)) + monkeypatch.setattr(lr, "_finalize_job", + lambda *a, **k: spy.__setitem__("finalize", k.get("result_ok"))) + + lr._monitor_agent(_FakeProc(0), run_id, "developer", "r", "feature/y", + output_path=str(log), log_fh=None, job_id=99) + + assert spy["post"] == 0 # no success comment + assert spy["advance"] == 0 # stage NOT advanced + assert spy["finalize"] is False # finalize told the result was invalid + assert spy["alert"] == 1 # empty-result alert fired + + +# =========================================================================== +# _handle_auth_marker — post-factum auth detection resets preflight cache (P1b) +# =========================================================================== +class TestAuthMarkerHandling: + def test_auth_marker_resets_preflight_cache(self, tmp_path, monkeypatch): + log = tmp_path / "auth.log" + log.write_text("Error: Not logged in. Please run /login\n") + reset = {"n": 0} + monkeypatch.setattr(preflight, "reset_cache", + lambda: reset.__setitem__("n", reset["n"] + 1)) + found = AgentLauncher()._handle_auth_marker(str(log)) + assert found is True + assert reset["n"] == 1 + + def test_no_auth_marker_no_reset(self, tmp_path, monkeypatch): + log = tmp_path / "plain.log" + log.write_text("Traceback: ValueError somewhere\n") + reset = {"n": 0} + monkeypatch.setattr(preflight, "reset_cache", + lambda: reset.__setitem__("n", reset["n"] + 1)) + found = AgentLauncher()._handle_auth_marker(str(log)) + assert found is False + assert reset["n"] == 0 diff --git a/tests/test_preflight_auth.py b/tests/test_preflight_auth.py new file mode 100644 index 0000000..403a557 --- /dev/null +++ b/tests/test_preflight_auth.py @@ -0,0 +1,246 @@ +"""ORCH-044 (P1): token-free preflight auth gate. + +`claude --version` answers even when claude is logged OUT, so version-only +preflight was blind to auth. These tests cover the new local credentials check: +missing / expired / valid token, broken JSON fail-safe, no network, caching, +HOME-correct path resolution, and the queue-worker claim gate. + +No real claude/Popen is spawned: `_run_version` is stubbed and credentials live +in tmp files. DB is a fresh per-test sqlite (mirrors tests/test_resilience.py). +""" +import os +import json +import socket +import tempfile + +import pytest + +_test_db = os.path.join(tempfile.gettempdir(), "test_orchestrator_preflight_auth.db") +os.environ["ORCH_DB_PATH"] = _test_db +os.environ["ORCH_REPOS_DIR"] = tempfile.gettempdir() +os.environ["ORCH_GITEA_TOKEN"] = "test-token" +os.environ["ORCH_PLANE_API_TOKEN"] = "test-token" + +import src.db as db +from src.db import init_db, enqueue_job, get_job, count_running_jobs +from src import preflight +from src.queue_worker import QueueWorker +from src.agents.launcher import AgentLauncher + + +@pytest.fixture(autouse=True) +def fresh_db(tmp_path, monkeypatch): + monkeypatch.setattr(db.settings, "db_path", str(tmp_path / "res.db")) + init_db() + preflight.reset_cache() + # auth check on by default; large TTL unless a test overrides it. + monkeypatch.setattr(preflight.settings, "preflight_check_auth", True) + yield + + +def _fake_bin(monkeypatch, tmp_path): + """A bin path that exists + a --version that always succeeds (auth-agnostic).""" + b = tmp_path / "claude" + b.write_text("#!/bin/sh\necho v1\n") + monkeypatch.setattr(preflight, "_claude_bin", lambda: str(b)) + monkeypatch.setattr(preflight, "_run_version", lambda b: (True, "1.2.3")) + + +def _write_creds(tmp_path, *, expires_ms=None, access_token="tok", oauth=True, + raw=None): + path = tmp_path / ".credentials.json" + if raw is not None: + path.write_text(raw) + return path + body = {} + if oauth: + oa = {"accessToken": access_token} + if expires_ms is not None: + oa["expiresAt"] = expires_ms + body["claudeAiOauth"] = oa + path.write_text(json.dumps(body)) + return path + + +# --------------------------------------------------------------------------- +# TC-01 / AC-1: not logged in (no credentials file) -> FAIL +# --------------------------------------------------------------------------- +def test_missing_credentials_fails(monkeypatch, tmp_path): + _fake_bin(monkeypatch, tmp_path) + monkeypatch.setattr(preflight, "_credentials_path", + lambda: str(tmp_path / "nope.json")) + ok, reason = preflight.check(force=True) + assert ok is False + assert "logged in" in reason.lower() or "credentials" in reason.lower() + + +# --------------------------------------------------------------------------- +# TC-02 / AC-2: expired OAuth token -> FAIL +# --------------------------------------------------------------------------- +def test_expired_token_fails(monkeypatch, tmp_path): + _fake_bin(monkeypatch, tmp_path) + past = (int(__import__("time").time()) - 3600) * 1000 # 1h ago, epoch ms + creds = _write_creds(tmp_path, expires_ms=past) + monkeypatch.setattr(preflight, "_credentials_path", lambda: str(creds)) + ok, reason = preflight.check(force=True) + assert ok is False + assert "expired" in reason.lower() + + +# --------------------------------------------------------------------------- +# TC-03 / AC-3: valid login -> OK (no regression) +# --------------------------------------------------------------------------- +def test_valid_login_ok(monkeypatch, tmp_path): + _fake_bin(monkeypatch, tmp_path) + future = (int(__import__("time").time()) + 3600) * 1000 # 1h ahead + creds = _write_creds(tmp_path, expires_ms=future) + monkeypatch.setattr(preflight, "_credentials_path", lambda: str(creds)) + ok, reason = preflight.check(force=True) + assert ok is True + + +def test_token_without_expiry_is_ok(monkeypatch, tmp_path): + # accessToken present but no expiresAt -> cannot prove expiry -> OK (ADR §P1.5). + _fake_bin(monkeypatch, tmp_path) + creds = _write_creds(tmp_path, expires_ms=None) + monkeypatch.setattr(preflight, "_credentials_path", lambda: str(creds)) + ok, _ = preflight.check(force=True) + assert ok is True + + +# --------------------------------------------------------------------------- +# TC-04 / AC-1: broken / unreadable credentials JSON -> FAIL (no exception) +# --------------------------------------------------------------------------- +def test_broken_json_fails_without_raising(monkeypatch, tmp_path): + _fake_bin(monkeypatch, tmp_path) + creds = _write_creds(tmp_path, raw="{ this is not valid json ") + monkeypatch.setattr(preflight, "_credentials_path", lambda: str(creds)) + ok, reason = preflight.check(force=True) # must not raise + assert ok is False + assert "logged in" in reason.lower() or "unreadable" in reason.lower() + + +def test_no_oauth_block_fails(monkeypatch, tmp_path): + _fake_bin(monkeypatch, tmp_path) + creds = _write_creds(tmp_path, oauth=False) + monkeypatch.setattr(preflight, "_credentials_path", lambda: str(creds)) + ok, reason = preflight.check(force=True) + assert ok is False + assert "oauth" in reason.lower() or "logged in" in reason.lower() + + +# --------------------------------------------------------------------------- +# TC-05 / AC-5: token-free — no network call in the auth path +# --------------------------------------------------------------------------- +def test_auth_check_makes_no_network_call(monkeypatch, tmp_path): + _fake_bin(monkeypatch, tmp_path) + future = (int(__import__("time").time()) + 3600) * 1000 + creds = _write_creds(tmp_path, expires_ms=future) + monkeypatch.setattr(preflight, "_credentials_path", lambda: str(creds)) + + def _no_net(*a, **k): + raise AssertionError("token-free auth check must not open a socket") + + monkeypatch.setattr(socket, "socket", _no_net) + ok, _ = preflight.check(force=True) + assert ok is True + + +# --------------------------------------------------------------------------- +# TC-06 / AC-6: auth result cached within preflight_cache_ttl +# --------------------------------------------------------------------------- +def test_auth_result_cached_within_ttl(monkeypatch, tmp_path): + _fake_bin(monkeypatch, tmp_path) + monkeypatch.setattr(preflight.settings, "preflight_cache_ttl", 999) + + calls = {"n": 0} + real = preflight._check_auth + + future = (int(__import__("time").time()) + 3600) * 1000 + creds = _write_creds(tmp_path, expires_ms=future) + monkeypatch.setattr(preflight, "_credentials_path", lambda: str(creds)) + + def counting(): + calls["n"] += 1 + return real() + + monkeypatch.setattr(preflight, "_check_auth", counting) + preflight.reset_cache() + preflight.check() # miss -> reads creds + preflight.check() # cached -> no re-read + preflight.check() + assert calls["n"] == 1 + + +# --------------------------------------------------------------------------- +# TC-07 / TR-1.3: credentials path resolves from AGENT_HOME, not process env +# --------------------------------------------------------------------------- +def test_credentials_path_follows_agent_home(monkeypatch, tmp_path): + agent_home = tmp_path / "agent_home" + agent_home.mkdir() + monkeypatch.setattr(AgentLauncher, "AGENT_HOME", str(agent_home)) + monkeypatch.setattr(preflight.settings, "claude_credentials_path", "") + # The orchestrator process HOME points somewhere else entirely. + monkeypatch.setenv("HOME", str(tmp_path / "orchestrator_home")) + + resolved = preflight._credentials_path() + assert resolved == str(agent_home / ".claude" / ".credentials.json") + assert str(tmp_path / "orchestrator_home") not in resolved + + +def test_explicit_credentials_path_wins(monkeypatch, tmp_path): + monkeypatch.setattr(preflight.settings, "claude_credentials_path", + str(tmp_path / "explicit.json")) + assert preflight._credentials_path() == str(tmp_path / "explicit.json") + + +# --------------------------------------------------------------------------- +# TC-08 / AC-4: auth-fail blocks the queue-worker claim +# --------------------------------------------------------------------------- +def test_worker_does_not_claim_when_auth_fails(monkeypatch, tmp_path): + _fake_bin(monkeypatch, tmp_path) + monkeypatch.setattr(preflight, "_credentials_path", + lambda: str(tmp_path / "missing.json")) # not logged in + called = {"launch": False} + monkeypatch.setattr("src.queue_worker.launcher.launch_job", + lambda job: called.__setitem__("launch", True)) + + jid = enqueue_job("analyst", "r") + w = QueueWorker(max_concurrency=1, poll_interval=0.01) + w._drain_once() + + assert called["launch"] is False + assert get_job(jid)["status"] == "queued" + assert count_running_jobs() == 0 + assert w.last_preflight_ok is False + assert "logged in" in w.last_preflight_reason.lower() \ + or "credentials" in w.last_preflight_reason.lower() + + +# --------------------------------------------------------------------------- +# Toggle off: preflight_check_auth=False keeps the old version-only behaviour +# --------------------------------------------------------------------------- +def test_auth_toggle_off_skips_check(monkeypatch, tmp_path): + _fake_bin(monkeypatch, tmp_path) + monkeypatch.setattr(preflight.settings, "preflight_check_auth", False) + monkeypatch.setattr(preflight, "_credentials_path", + lambda: str(tmp_path / "missing.json")) + ok, _ = preflight.check(force=True) + assert ok is True # auth not consulted -> version-only pass + + +# --------------------------------------------------------------------------- +# is_auth_failure_text: post-factum marker detection (P1b) +# --------------------------------------------------------------------------- +@pytest.mark.parametrize("text", [ + "Error: Not logged in. Please run /login", + "401 Unauthorized", + "invalid api key provided", +]) +def test_is_auth_failure_text_positive(text): + assert preflight.is_auth_failure_text(text) is True + + +@pytest.mark.parametrize("text", ["", "429 rate limit", "Traceback ValueError"]) +def test_is_auth_failure_text_negative(text): + assert preflight.is_auth_failure_text(text) is False diff --git a/tests/test_resilience.py b/tests/test_resilience.py index 1d72117..5ed28a1 100644 --- a/tests/test_resilience.py +++ b/tests/test_resilience.py @@ -37,6 +37,17 @@ def fresh_db(tmp_path, monkeypatch): # A. Preflight # --------------------------------------------------------------------------- class TestPreflight: + @pytest.fixture(autouse=True) + def _isolate_auth_gate(self, monkeypatch): + # ORCH-044: preflight.check() also runs a token-free auth gate reading + # /.claude/.credentials.json (AgentLauncher.AGENT_HOME, not the + # process HOME). In a clean CI runner those creds are absent, so the gate + # returns (False, ...) and version-branch assertions would fail for purely + # environmental reasons. Stub the gate green; auth is covered by + # tests/test_preflight_auth.py. Production default (preflight_check_auth=True) + # is unchanged. + monkeypatch.setattr(preflight, "_check_auth", lambda: (True, "auth ok (test stub)")) + def test_fail_when_bin_missing(self, monkeypatch): monkeypatch.setattr(preflight, "_claude_bin", lambda: "/no/such/claude") ok, reason = preflight.check(force=True)