feat(qg): ORCH-045 — poll check_ci_green with retry to fix CI race (pending->success)
This commit is contained in:
@@ -5,6 +5,7 @@
|
||||
## [Unreleased]
|
||||
|
||||
### Added
|
||||
- **Поллинг с ретраем в 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 <T>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-`<a>`-ссылки — на `docs/work-items/<WI>/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`.
|
||||
- **Конфигурируемые модель LLM и режим работы (`--effort`) агентов** (ORCH-41): модель/effort каждого агента вынесены из хардкода `launcher.py` в конфиг — глобально per-agent (`ORCH_AGENT_MODEL_<AGENT>` / `ORCH_AGENT_EFFORT_<AGENT>`, дефолты `ORCH_AGENT_MODEL_DEFAULT=claude-opus-4-8`, `ORCH_AGENT_EFFORT_DEFAULT=high`) и per-project (`agent_models` / `agent_efforts` в `ORCH_PROJECTS_JSON`). Резолверы `resolve_agent_model` / `resolve_agent_effort` (приоритет project > per-agent env > default > пусто), валидация effort `{low,medium,high,xhigh,max}`, опц. `ORCH_AGENT_FALLBACK_MODEL` (`--fallback-model`). Хардкод `"model":"opus"` (architect/reviewer) удалён. Тесты: `test_resolve_agent_model.py`, `test_resolve_agent_effort.py`.
|
||||
- **Единый status-коммент агентов в Plane** (ORCH-016): `usage.build_status_comment(...)` — один хелпер для ВСЕХ ролей (analyst..deployer). HTML-формат: header `{icon} {Role} — {описание}`, опциональная строка `Verdict/Status: …` из YAML-frontmatter артефакта, **строка `Длительность: 4m 12s`** (явный `duration_s` от launcher, fallback из `agent_runs` для аналитика), `<b>Документы:</b><ul><li><a>…</a></li></ul>`, тех-хвост `<sub>tokens · cost</sub>`. Утилитки: `usage.fmt_duration`, `usage.get_agent_duration`, новый модуль `src/frontmatter.py` (defensive YAML reader). ADR `docs/work-items/ORCH-016/06-adr/ADR-001-unified-status-comment.md`.
|
||||
|
||||
@@ -8,6 +8,7 @@ Per-work-item решения живут в `docs/work-items/<id>/06-adr/ADR-NNN-
|
||||
| adr-0001 | Реестр проектов (multi-repo) | accepted | 2026-06-02 | ORCH-6 |
|
||||
| adr-0002 | Очередь задач вместо in-process потоков | accepted | 2026-06-03 | ORCH-1 |
|
||||
| adr-0003 | Условный staging-гейт перед прод-деплоем | accepted | 2026-06-05 | ORCH-35 |
|
||||
| adr-0004 | Поллинг с ретраем в check_ci_green (фикс CI-race) | accepted | 2026-06-05 | ORCH-045 |
|
||||
|
||||
## Формат
|
||||
**Контекст → Решение → Альтернативы → Последствия → Связи.** Статус: proposed / accepted / superseded.
|
||||
|
||||
45
docs/architecture/adr/adr-0004-ci-poll-retry.md
Normal file
45
docs/architecture/adr/adr-0004-ci-poll-retry.md
Normal file
@@ -0,0 +1,45 @@
|
||||
# adr-0004: Поллинг с ретраем в quality-gate check_ci_green (фикс CI-race)
|
||||
|
||||
- **Статус:** accepted
|
||||
- **Дата:** 2026-06-05
|
||||
- **Задача:** ORCH-045
|
||||
|
||||
## Контекст
|
||||
Quality-gate `check_ci_green(repo, branch)` (`src/qg/checks.py`) проверяет combined commit-status ветки через Gitea API сразу после того, как developer-агент запушил код. Реализация была **single-shot**: один `GET /repos/{owner}/{repo}/commits/{branch}/status`, чтение `data["state"]` — `success` → пропуск, иначе → сразу `False`.
|
||||
|
||||
Это создавало race condition. Gitea-CI после пуша 1–3 секунды держит combined state `pending`, пока не отработают чек-раннеры. Если гейт опрашивал статус в этом окне, он получал `pending` и возвращал `False` **ровно один раз** — повторного опроса не было. Combined state затем дозеленевал до `success`, но гейт уже промахнулся, и задача застревала насмерть без видимой причины.
|
||||
|
||||
Реальный инцидент **ORCH-017**: гейт опросил статус в 17:58:54 → `pending`; CI дозеленел в 17:58:55. Задача встала в тупик (см. `docs/history` / lessons ORCH-017).
|
||||
|
||||
## Решение
|
||||
`check_ci_green` превращён из single-shot в **polling с ретраем**:
|
||||
|
||||
- `state == "success"` → `(True, "CI green")` немедленно.
|
||||
- `state in ("failure", "error")` → `(False, "CI state: <state>")` немедленно — CI красный, ретрай бессмыслен (терминальное состояние).
|
||||
- `state == "pending"` (или `unknown` / иное не-терминальное) → `time.sleep(interval)` и опрос снова, до `N` попыток.
|
||||
- После исчерпания всех попыток при всё ещё `pending` → `(False, "CI still pending after <T>s")` — **явный** провал с причиной, чтобы оператор видел тупик, а не молчаливый стол.
|
||||
- `404` → `(False, "Branch ... not found or no status")` — как раньше.
|
||||
- Транзиентная `httpx.HTTPError` на отдельной попытке — **не падаем сразу**: логируем и пробуем ещё в рамках лимита попыток; если все попытки — сетевая ошибка → `(False, "API error: <e>")`.
|
||||
|
||||
Параметры вынесены в `src/config.py` (pydantic-settings, env-prefix `ORCH_`, единый стиль с остальными настройками):
|
||||
- `ci_poll_max_attempts` (env `ORCH_CI_POLL_MAX_ATTEMPTS`, дефолт **12**)
|
||||
- `ci_poll_interval_s` (env `ORCH_CI_POLL_INTERVAL_S`, дефолт **10**)
|
||||
|
||||
Итого по умолчанию гейт ждёт `pending` до ~2 минут (12 × 10s) перед тем как явно провалиться. Каждая не-финальная попытка логируется через существующий `logger` (`check_ci_green: attempt i/N, state=..., retrying in Ns`). `timeout=10` на каждый отдельный запрос сохранён.
|
||||
|
||||
Сигнатура `check_ci_green(repo, branch) -> tuple[bool, str]` **не менялась** — её зовёт stage_engine и реестр гейтов `QG_CHECKS`.
|
||||
|
||||
## Альтернативы
|
||||
- **Оставить single-shot, опрашивать гейт повторно снаружи (на уровне stage_engine/воркера).** Отклонено: размазывает логику CI-ожидания по слоям, дублирует таймауты; гейт — естественное место знания о combined-status.
|
||||
- **Webhook от Gitea на завершение CI вместо поллинга.** Отложено: требует надёжной доставки/дедупликации вебхуков именно по CI-статусу и переписывания триггера стадии; поллинг — минимальный, локализованный фикс race-а здесь и сейчас.
|
||||
- **Бесконечный ретрай до зелёного.** Отклонено: задача могла бы висеть вечно при реально зависшем CI; ограниченный бюджет + явный `False` с причиной даёт оператору сигнал.
|
||||
|
||||
## Последствия
|
||||
- CI-race ORCH-017 закрыт: транзиентный `pending` переживается ретраем, гейт не промахивается.
|
||||
- `check_ci_green` теперь **блокирующий** до ~`max_attempts × interval` секунд при затяжном `pending` (по умолчанию ~2 мин). Это осознанный trade-off; для красного CI и success — выход немедленный, без задержки.
|
||||
- Тупик больше не молчаливый: истечение попыток → `(False, "CI still pending after <T>s")`, причина видна.
|
||||
- Бюджет/интервал настраиваемы через env без правки кода.
|
||||
- `check_tests_passed` / `_parse_tests_verdict` (ORCH-47) **не затронуты**.
|
||||
|
||||
## Связи
|
||||
ORCH-017 (инцидент-первоисточник: deadlock shared-gate из-за CI-race), реестр гейтов `QG_CHECKS` (`check_ci_green`), стадия `development`. Тесты: `tests/test_qg.py::TestCheckCIGreen`.
|
||||
@@ -121,6 +121,15 @@ class Settings(BaseSettings):
|
||||
log_keep_max: int = 500
|
||||
|
||||
|
||||
# ORCH-045: quality-gate CI poll/retry. check_ci_green polls the Gitea
|
||||
# combined commit status up to ci_poll_max_attempts times, sleeping
|
||||
# ci_poll_interval_s between attempts, to ride out a transient pending
|
||||
# state right after the developer push (race fix, see ORCH-017).
|
||||
# ci_poll_max_attempts -> max status polls (env ORCH_CI_POLL_MAX_ATTEMPTS)
|
||||
# ci_poll_interval_s -> seconds between polls (env ORCH_CI_POLL_INTERVAL_S)
|
||||
ci_poll_max_attempts: int = 12
|
||||
ci_poll_interval_s: int = 10
|
||||
|
||||
# Telegram notifications
|
||||
telegram_bot_token: str = ""
|
||||
telegram_chat_id: str = ""
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
"""Quality Gate checks — real implementations using Gitea/Plane API and filesystem."""
|
||||
|
||||
import os
|
||||
import time
|
||||
import logging
|
||||
import subprocess
|
||||
import httpx
|
||||
@@ -82,23 +83,65 @@ def check_ci_green(repo: str, branch: str) -> tuple[bool, str]:
|
||||
"""
|
||||
Check if CI status is green for branch via Gitea API.
|
||||
GET /repos/{owner}/{repo}/commits/{branch}/status
|
||||
|
||||
ORCH-045: polling with retry to fix a race condition. The gate used to do a
|
||||
single status read right after the developer push; if CI was still ``pending``
|
||||
for the first 1-3s (real case ORCH-017: polled 17:58:54 -> pending, CI went
|
||||
green 17:58:55) the gate returned False once and the task stalled silently.
|
||||
|
||||
Behaviour now:
|
||||
* ``success`` -> (True, "CI green") immediately.
|
||||
* ``failure`` / ``error`` -> (False, "CI state: <state>") immediately
|
||||
(CI is red, retrying is pointless).
|
||||
* ``pending`` / unknown -> sleep ``ci_poll_interval_s`` and poll again,
|
||||
up to ``ci_poll_max_attempts`` times.
|
||||
* still pending after all attempts -> (False, "CI still pending after <T>s").
|
||||
* 404 -> (False, "Branch not found or no status").
|
||||
* transient httpx errors -> logged and retried within the attempt budget;
|
||||
if every attempt errors -> (False, "API error: <e>").
|
||||
"""
|
||||
owner = settings.gitea_owner
|
||||
url = f"{GITEA_BASE}/repos/{owner}/{repo}/commits/{branch}/status"
|
||||
|
||||
try:
|
||||
resp = httpx.get(url, headers=GITEA_HEADERS, timeout=10)
|
||||
if resp.status_code == 404:
|
||||
return False, f"Branch '{branch}' not found or no status"
|
||||
resp.raise_for_status()
|
||||
data = resp.json()
|
||||
state = data.get("state", "unknown")
|
||||
if state == "success":
|
||||
return True, "CI green"
|
||||
return False, f"CI state: {state}"
|
||||
except httpx.HTTPError as e:
|
||||
logger.error(f"Gitea API error checking CI: {e}")
|
||||
return False, f"API error: {e}"
|
||||
attempts = settings.ci_poll_max_attempts
|
||||
interval = settings.ci_poll_interval_s
|
||||
last_state = "unknown"
|
||||
last_error: Exception | None = None
|
||||
|
||||
for i in range(1, attempts + 1):
|
||||
try:
|
||||
resp = httpx.get(url, headers=GITEA_HEADERS, timeout=10)
|
||||
if resp.status_code == 404:
|
||||
return False, f"Branch '{branch}' not found or no status"
|
||||
resp.raise_for_status()
|
||||
data = resp.json()
|
||||
last_state = data.get("state", "unknown")
|
||||
last_error = None
|
||||
|
||||
if last_state == "success":
|
||||
return True, "CI green"
|
||||
if last_state in ("failure", "error"):
|
||||
return False, f"CI state: {last_state}"
|
||||
# non-terminal (pending / unknown / other) -> retry below
|
||||
except httpx.HTTPError as e:
|
||||
last_error = e
|
||||
logger.error(f"check_ci_green: attempt {i}/{attempts} API error: {e}")
|
||||
|
||||
if i < attempts:
|
||||
if last_error is not None:
|
||||
logger.info(
|
||||
f"check_ci_green: attempt {i}/{attempts}, error, retrying in {interval}s"
|
||||
)
|
||||
else:
|
||||
logger.info(
|
||||
f"check_ci_green: attempt {i}/{attempts}, state={last_state}, "
|
||||
f"retrying in {interval}s"
|
||||
)
|
||||
time.sleep(interval)
|
||||
|
||||
if last_error is not None:
|
||||
return False, f"API error: {last_error}"
|
||||
return False, f"CI still pending after {attempts * interval}s"
|
||||
|
||||
|
||||
def check_review_approved(repo: str, pr_number: int) -> tuple[bool, str]:
|
||||
|
||||
@@ -93,38 +93,82 @@ class TestCheckArchitectureDone:
|
||||
assert passed is False
|
||||
|
||||
|
||||
def _ci_status_resp(state, status_code=200):
|
||||
"""Build a MagicMock httpx response for the Gitea combined-status endpoint."""
|
||||
mock_resp = MagicMock()
|
||||
mock_resp.status_code = status_code
|
||||
mock_resp.json.return_value = {"state": state}
|
||||
mock_resp.raise_for_status = MagicMock()
|
||||
return mock_resp
|
||||
|
||||
|
||||
class TestCheckCIGreen:
|
||||
"""ORCH-045: check_ci_green now polls with retry to ride out a transient
|
||||
`pending` right after the developer push (race fix, see ORCH-017)."""
|
||||
|
||||
@patch("src.qg.checks.time.sleep")
|
||||
@patch("src.qg.checks.httpx.get")
|
||||
def test_ci_success(self, mock_get):
|
||||
mock_resp = MagicMock()
|
||||
mock_resp.status_code = 200
|
||||
mock_resp.json.return_value = {"state": "success"}
|
||||
mock_resp.raise_for_status = MagicMock()
|
||||
mock_get.return_value = mock_resp
|
||||
def test_ci_success_first_attempt(self, mock_get, mock_sleep):
|
||||
mock_get.return_value = _ci_status_resp("success")
|
||||
|
||||
passed, reason = check_ci_green("enduro-trails", "feature/ET-001-test")
|
||||
assert passed is True
|
||||
assert "green" in reason.lower()
|
||||
assert mock_get.call_count == 1
|
||||
mock_sleep.assert_not_called()
|
||||
|
||||
@patch("src.qg.checks.time.sleep")
|
||||
@patch("src.qg.checks.httpx.get")
|
||||
def test_ci_pending(self, mock_get):
|
||||
mock_resp = MagicMock()
|
||||
mock_resp.status_code = 200
|
||||
mock_resp.json.return_value = {"state": "pending"}
|
||||
mock_resp.raise_for_status = MagicMock()
|
||||
mock_get.return_value = mock_resp
|
||||
def test_ci_pending_then_success(self, mock_get, mock_sleep):
|
||||
# pending on the 1st poll, green on the 2nd -> success after one retry.
|
||||
mock_get.side_effect = [
|
||||
_ci_status_resp("pending"),
|
||||
_ci_status_resp("success"),
|
||||
]
|
||||
|
||||
passed, reason = check_ci_green("enduro-trails", "feature/ET-001-test")
|
||||
assert passed is True
|
||||
assert "green" in reason.lower()
|
||||
assert mock_get.call_count == 2
|
||||
assert mock_sleep.call_count == 1 # slept once between the two polls
|
||||
|
||||
@patch("src.qg.checks.time.sleep")
|
||||
@patch("src.qg.checks.httpx.get")
|
||||
def test_ci_failure_no_retry(self, mock_get, mock_sleep):
|
||||
# CI is red -> terminal, return immediately without sleeping/retrying.
|
||||
mock_get.return_value = _ci_status_resp("failure")
|
||||
|
||||
passed, reason = check_ci_green("enduro-trails", "feature/ET-001-test")
|
||||
assert passed is False
|
||||
assert "failure" in reason
|
||||
assert mock_get.call_count == 1
|
||||
mock_sleep.assert_not_called()
|
||||
|
||||
@patch("src.qg.checks.time.sleep")
|
||||
@patch("src.qg.checks.httpx.get")
|
||||
def test_ci_branch_not_found(self, mock_get):
|
||||
def test_ci_pending_exhausts_attempts(self, mock_get, mock_sleep):
|
||||
# Always pending -> after ci_poll_max_attempts polls return an explicit
|
||||
# (False, "...pending...") so the operator sees the reason (no silent stall).
|
||||
from src.qg.checks import settings as checks_settings
|
||||
|
||||
mock_get.return_value = _ci_status_resp("pending")
|
||||
|
||||
passed, reason = check_ci_green("enduro-trails", "feature/ET-001-test")
|
||||
assert passed is False
|
||||
assert "pending" in reason.lower()
|
||||
assert mock_get.call_count == checks_settings.ci_poll_max_attempts
|
||||
|
||||
@patch("src.qg.checks.time.sleep")
|
||||
@patch("src.qg.checks.httpx.get")
|
||||
def test_ci_branch_not_found(self, mock_get, mock_sleep):
|
||||
mock_resp = MagicMock()
|
||||
mock_resp.status_code = 404
|
||||
mock_get.return_value = mock_resp
|
||||
|
||||
passed, reason = check_ci_green("enduro-trails", "nonexistent")
|
||||
assert passed is False
|
||||
assert "not found" in reason.lower()
|
||||
assert mock_get.call_count == 1
|
||||
|
||||
|
||||
class TestCheckReviewApproved:
|
||||
|
||||
Reference in New Issue
Block a user