diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b1e1bc..c008b1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ## [Unreleased] ### Added +- **Дословный текст 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`. - **Конфигурируемые модель LLM и режим работы (`--effort`) агентов** (ORCH-41): модель/effort каждого агента вынесены из хардкода `launcher.py` в конфиг — глобально per-agent (`ORCH_AGENT_MODEL_` / `ORCH_AGENT_EFFORT_`, дефолты `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`. diff --git a/docs/architecture/README.md b/docs/architecture/README.md index 4baebe5..9fdfe85 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -7,6 +7,7 @@ - **Webhook Receivers** (`src/webhooks/plane.py`, `gitea.py`) — приём событий, HMAC-проверка, дедупликация (`_dedup.py`). Роуты: `POST /webhook/plane`, `POST /webhook/gitea`. - **State Machine** (`src/stages.py`) — `STAGE_TRANSITIONS`: переходы, агент и QG каждой стадии. Хелперы: `get_next_stage`, `get_agent_for_stage`, `get_qg_for_stage`, `get_previous_stage`. - **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. @@ -46,6 +47,13 @@ created → analysis → architecture → development → review → testing → - Deploy / deploy-staging FAILED → откат на `development`. - `get_previous_stage` использует порядок ключей `STAGE_TRANSITIONS`. +### Обогащение `task_desc` при заворотах (ORCH-046) +При откате на `development` `task_desc` (попадает в `.task-dev.md` developer-агента) несёт **дословный must-fix текст**, а не только ссылку — чтобы агент видел суть претензий сразу и не повторял ту же ошибку: +- **reviewer REQUEST_CHANGES** → дословные пункты P0/P1 из секции `## Findings` файла `12-review.md` (`extract_review_findings`); +- **tester `check_tests_passed` FAIL** → `reason` гейта + фрагмент тела `13-test-report.md` (приоритет: `## Вывод pytest` → FAIL-строки `## Результаты` → `## Итог`; `extract_test_failures`). + +Ссылка на полный файл-артефакт сохраняется всегда («Полный контекст»). Парсеры `src/review_parse.py` — defensive (never-raise); при отсутствующем/битом артефакте `task_desc` graceful-фоллбэк на прежнюю ссылку-строку, последовательность отката и retry-счётчик не меняются (ADR `docs/work-items/ORCH-046/06-adr/ADR-001-embed-findings-in-task-desc.md`). + ### Plane Sync: единый status-коммент агентов (ORCH-016) Все агенты (analyst / architect / developer / reviewer / tester / deployer) пишут финальный коммент через **один хелпер** `usage.build_status_comment(...)` (ADR `docs/work-items/ORCH-016/06-adr/ADR-001-unified-status-comment.md`). Формат HTML, разделители `
`: diff --git a/docs/work-items/ORCH-046/00-business-request.md b/docs/work-items/ORCH-046/00-business-request.md new file mode 100644 index 0000000..829c9df --- /dev/null +++ b/docs/work-items/ORCH-046/00-business-request.md @@ -0,0 +1,7 @@ +# Business Request: stage_engine: pass reviewer/tester findings text to developer (not just link) + +Work Item ID: ORCH-046 + +## Description + +TBD diff --git a/docs/work-items/ORCH-046/01-brd.md b/docs/work-items/ORCH-046/01-brd.md new file mode 100644 index 0000000..a8070e6 --- /dev/null +++ b/docs/work-items/ORCH-046/01-brd.md @@ -0,0 +1,86 @@ +# BRD — ORCH-046: pass reviewer/tester findings text to developer (not just link) + +Work Item ID: ORCH-046 +Stage: analysis +Author: analyst +Date: 2026-06-06 + +## 1. Контекст и проблема + +Оркестратор при заворотах задачи деву (откат на `development`) формирует +описание задачи (`task_desc`), которое попадает в `.task-dev.md` запускаемого +агента-разработчика. Сейчас в двух ветках отката этот текст содержит **только +ссылку на файл-артефакт**, без сути замечаний: + +- **Reviewer → REQUEST_CHANGES** (`src/stage_engine.py`, ветка + `_handle_qg_failure_rollbacks`, ~стр. 419): `task_desc` = + `"…Fix findings in docs/work-items//12-review.md"`. +- **Tester → FAIL** (`check_tests_passed`, ~стр. 455): `task_desc` = + `"…Fix failures described in docs/work-items//13-test-report.md"`. + +В результате developer-агент получает инструкцию «иди читай файл». Ключевые +претензии (P0/P1 у ревьюера, причина падения у тестера) часто проскакивают — +агент не открывает файл целиком или теряет фокус, повторяет ту же ошибку, и +задача снова заворачивается. Это «испорченный телефон»: расход циклов retry +(`MAX_DEVELOPER_RETRIES = 3`), деньги на токены, простой конвейера. + +## 2. Бизнес-цель + +Убрать «испорченный телефон» между reviewer/tester и developer при заворотах: +встраивать **дословный текст ключевых замечаний** прямо в `task_desc`, чтобы +developer-агент видел суть претензий сразу, а не только ссылку. + +Это снижает число повторных заворотов и расход retry-бюджета на одну задачу. + +## 3. Объём (вариант A — выбран Славой 06.06) + +Минимальное, низкорисковое изменение **ядра** (`stage_engine`), которое: + +1. Извлекает из `12-review.md` блок findings и выносит **must-fix (P0/P1) + дословно** в `task_desc` при reviewer REQUEST_CHANGES. +2. Извлекает из `13-test-report.md` причину FAIL (reason из гейта + релевантный + фрагмент тела отчёта) в `task_desc` при tester FAIL. +3. Во всех случаях **сохраняет ссылку на полный файл** как дополнительный + контекст («полный контекст — см. файл»). +4. Извлечение выполняется новым отдельным хелпером-парсером + (`src/review_parse.py`), который **никогда не бросает исключение**: при + отсутствующем/битом файле возвращает пустой результат, и вызывающий код + делает graceful fallback на прежнюю ссылку-строку. + +## 4. Что НЕ входит в объём (out of scope) + +- НЕ трогать гейты `check_*` (в т. ч. ORCH-45 `check_ci_green`, + ORCH-47 `_parse_tests_verdict`) — они в проде, поведение неизменно. +- НЕ трогать реестр `QG_CHECKS`. +- НЕ менять сигнатуры публичных функций (`advance_stage`, `_run_qg`, + `check_*`). +- НЕ менять webhook-пути. +- НЕ менять retry-счётчик (`_developer_retry_count`, `MAX_DEVELOPER_RETRIES`) + и rollback-логику (последовательность `update_task_stage` → + `notify_stage_change` → `plane_notify_stage` → enqueue) — поведение + идентично. +- НЕ менять формат Plane-комментариев (`build_status_comment`). + +## 5. Заинтересованные стороны + +- **Owner (Слава)** — заказчик, выбрал вариант A. +- **Developer-агенты** — потребители `task_desc`: получают суть замечаний. +- **Конвейер всех проектов** (self-hosting) — выигрывает за счёт меньшего + числа заворотов. + +## 6. Ограничения и риски (self-hosting) + +- Правка ядра `stage_engine` — компонент крутится в продакшене и обслуживает + все проекты из общего инстанса/БД/очереди. Любая регрессия в формировании + `task_desc` или (тем более) исключение в `advance_stage` останавливает + конвейер всех проектов → **парсер обязан быть полностью graceful**. +- Обязателен прогон `deploy-staging` (8501) перед прод-деплоем. +- Это правка ядра → требуется ADR (per-work-item). + +## 7. Критерий успеха (бизнес) + +- При заворотах в `.task-dev.md` есть дословный текст ключевых замечаний + (P0/P1 ревьюера; reason+фрагмент тестера) плюс ссылка на полный файл. +- Парсер устойчив к битым/отсутствующим артефактам (graceful fallback на + старую ссылку-строку). +- Существующие тесты зелёные; поведение retry/rollback не изменилось. diff --git a/docs/work-items/ORCH-046/02-trz.md b/docs/work-items/ORCH-046/02-trz.md new file mode 100644 index 0000000..b6d6340 --- /dev/null +++ b/docs/work-items/ORCH-046/02-trz.md @@ -0,0 +1,209 @@ +# ТЗ — ORCH-046: встраивание текста findings reviewer/tester в task_desc + +Work Item ID: ORCH-046 +Stage: analysis +Author: analyst +Date: 2026-06-06 + +> Вариант A (минимальный, низкий риск). Это правка ЯДРА — обязателен ADR +> (per-work-item, `docs/work-items/ORCH-046/06-adr/`). + +## 1. Задействованные модули `src/` + +| Модуль | Изменение | +|--------|-----------| +| `src/review_parse.py` | **НОВЫЙ** хелпер-парсер: `extract_review_findings(path) -> str`, `extract_test_failures(path) -> str`. | +| `src/stage_engine.py` | Две ветки в `_handle_qg_failure_rollbacks`: reviewer REQUEST_CHANGES (~стр. 419) и tester `check_tests_passed` FAIL (~стр. 455) — встраивают извлечённый текст в `task_desc`. | + +Источники-образцы (не менять, использовать как референс паттерна «never raise» и +формата артефактов): +- `src/qg/checks.py::_parse_tests_verdict` — образец «never raise», split по `---`, `yaml.safe_load`. +- `src/frontmatter.py::read_frontmatter_value` — образец defensive-парсера. +- `.openclaw/agents/reviewer.md` — канонический формат `12-review.md`. +- `.openclaw/agents/tester.md` — канонический формат `13-test-report.md`. + +## 2. Новый модуль `src/review_parse.py` + +### 2.1. `extract_review_findings(path: str) -> str` + +Назначение: вернуть **дословный** текст must-fix findings (P0 + P1) из +`12-review.md` для встраивания в `task_desc`. + +Формат входного файла (канон reviewer.md, секция `## Findings`): + +```markdown +## Findings + +### P0 — Blocker +- [ ] <описание> + +### P1 — Must fix +- [ ] <описание> + +### P2 — Should fix +- [ ] <описание> +``` + +Требования к реализации: + +1. **Никогда не бросает исключение.** Любая ошибка (нет файла, IOError, кривой + markdown, нет секции `## Findings`) → возврат `""` (пустая строка). +2. Парсит **только** подсекции P0 и P1 (must-fix). P2/P3 игнорируются. +3. Заголовки подсекций распознаются устойчиво к регистру и к тире/дефису: + соответствие по наличию токена `P0` / `P1` в строке-заголовке уровня `###`. +4. Из распознанных подсекций берётся текст до следующего заголовка `###`/`##` + (т. е. тело подсекции дословно: пункты списка `- [ ] …` / `- …`). +5. Пустые подсекции (нет содержательных пунктов, только `(если есть)`-плейсхолдер + или ничего) — пропускаются. Если ни одного содержательного P0/P1 пункта нет + → возврат `""`. +6. Результат — компактный многострочный текст, пригодный для вставки в + `task_desc` (например, заголовок подсекции + её пункты). Длина результата + ограничивается разумным лимитом (`MAX_FINDINGS_CHARS`, напр. 2000) с + усечением и маркером `…(truncated)`; полный контекст всё равно остаётся в + файле. +7. Frontmatter (верхний `--- … ---`) при необходимости отбрасывается, чтобы не + попасть в тело; парсинг секции делается по телу markdown. + +Сигнатура и контракт (стабильны): +```python +def extract_review_findings(path: str) -> str: + """Дословный текст P0/P1 findings из 12-review.md. Never raises; '' при ошибке/пусто.""" +``` + +### 2.2. `extract_test_failures(path: str) -> str` + +Назначение: вернуть текст причины падения тестов из `13-test-report.md` для +встраивания в `task_desc`. + +Формат входного файла (канон tester.md): frontmatter `result: PASS|FAIL`, далее +тело с секциями `## Результаты` (таблица TC), `## Вывод pytest`, `## Итог`. + +Требования к реализации: + +1. **Никогда не бросает исключение.** Любая ошибка → возврат `""`. +2. Извлекает релевантный фрагмент тела, помогающий понять причину FAIL. + Приоритет источников (берём первый непустой): + - секция `## Вывод pytest` (вывод прогона — где видно упавшие тесты), и/или + - строки таблицы `## Результаты`, содержащие `FAIL`, и/или + - секция `## Итог`. +3. Результат усекается до `MAX_FAILURES_CHARS` (напр. 2000) с маркером + `…(truncated)`. +4. Если ничего извлечь не удалось → возврат `""` (вызывающий код делает + fallback на ссылку). + +> Примечание: «reason» из самого гейта (`check_tests_passed` → второй элемент +> кортежа) у вызывающего кода уже есть (`reason`) — он добавляется в `task_desc` +> вызывающим кодом (как и сейчас в комментарии тестера). `extract_test_failures` +> добавляет **фрагмент тела отчёта** поверх этого reason. + +Сигнатура и контракт (стабильны): +```python +def extract_test_failures(path: str) -> str: + """Релевантный фрагмент тела 13-test-report.md (причина FAIL). Never raises; '' при ошибке/пусто.""" +``` + +### 2.3. Общие требования модуля + +- Модуль логирует диагностические сообщения на уровне `logger.debug` + (`logging.getLogger("orchestrator.review_parse")`), как `frontmatter.py`. +- Никаких сетевых вызовов, только чтение файла с диска. +- Константы лимитов вынесены модульными (`MAX_FINDINGS_CHARS`, + `MAX_FAILURES_CHARS`). + +## 3. Изменения `src/stage_engine.py` + +### 3.1. Ветка reviewer REQUEST_CHANGES (внутри `_handle_qg_failure_rollbacks`) + +Текущее (~стр. 418–424): +```python +task_desc = ( + f"Work item: {work_item_id}\nRepo: {repo}\nBranch: {branch}\n" + f"Stage: development\nNote: REQUEST_CHANGES from reviewer " + f"(attempt {retry_count+1}/3). Fix findings in " + f"docs/work-items/{work_item_id}/12-review.md" +) +``` + +Целевое поведение: +- Сформировать путь к `12-review.md` через `get_worktree_path(repo, branch)` + + `docs/work-items/{work_item_id}/12-review.md` (как в `_check_review_approved_by_branch`). +- Вызвать `extract_review_findings(path)`. +- Если результат непустой — встроить findings **дословно** в `task_desc` + (под подзаголовком, напр. `Findings (P0/P1):\n`), а ссылку на файл + оставить как «полный контекст» (`Полный контекст: docs/work-items//12-review.md`). +- Если результат пустой (graceful fallback) — `task_desc` остаётся **как + сейчас** (ссылка-строка). Никаких исключений. +- Префиксная часть (`Work item / Repo / Branch / Stage / Note: REQUEST_CHANGES … + (attempt N/3)`) сохраняется без изменений. + +### 3.2. Ветка tester FAIL (`check_tests_passed`, внутри `_handle_qg_failure_rollbacks`) + +Текущее (~стр. 454–459): +```python +task_desc = ( + f"Work item: {work_item_id}\nRepo: {repo}\nBranch: {branch}\n" + f"Stage: development\nNote: Tests FAILED. " + f"Fix failures described in docs/work-items/{work_item_id}/13-test-report.md" +) +``` + +Целевое поведение: +- Сформировать путь к `13-test-report.md` аналогично. +- Вызвать `extract_test_failures(path)`. +- В `task_desc` всегда включить `reason` (он уже доступен в этой ветке — + передаётся в `_handle_qg_failure_rollbacks`). +- Если фрагмент тела непустой — встроить его дословно + (`Причина: {reason}\nДетали:\n`), плюс ссылку на файл как полный + контекст. +- Если фрагмент пустой — `task_desc` содержит `reason` + ссылку (graceful + fallback, не хуже текущего поведения). Никаких исключений. +- Префиксная часть и существующий Plane-комментарий тестера + (`❌ Тесты не прошли: {reason}…`) НЕ меняются. + +### 3.3. Инварианты (НЕ менять поведение) + +- Последовательность rollback в обеих ветках: `update_task_stage(task_id, + "development")` → `notify_stage_change` → `plane_notify_stage` → + (`set_issue_in_progress` для тестера) → проверка `_developer_retry_count` < + `MAX_DEVELOPER_RETRIES` → `enqueue_job("developer", …)` либо + `send_telegram` alert. Порядок и условия идентичны. +- `result.rolled_back_to`, `result.enqueued_agent`, `result.enqueued_job_id`, + `result.alerted` выставляются как сейчас. +- Меняется **только содержимое строки `task_desc`**, передаваемой в + `enqueue_job`. +- Импорт нового модуля — `from .review_parse import extract_review_findings, + extract_test_failures` в шапке `stage_engine.py`. + +## 4. Изменения API + +Нет. Публичные HTTP-эндпоинты (`/health`, `/status`, `/queue`, +`/webhook/plane`, `/webhook/gitea`) не затрагиваются. + +## 5. Изменения схемы БД + +Нет. Таблицы `tasks`, `agent_runs`, `jobs`, `events` не меняются. +`enqueue_job` вызывается с прежней сигнатурой. + +## 6. Требования к новым QG checks + +Нет. Реестр `QG_CHECKS` и все `check_*` не трогаются (явно out of scope). + +## 7. Артефакты pipeline (создать/обновить в этом PR) + +- `src/review_parse.py` — новый модуль. +- `tests/test_review_parse.py` — юнит-тесты парсера (см. 04-test-plan.yaml). +- Возможные дополнения в `tests/test_stage_engine.py` — проверка встраивания + текста в `task_desc` (rollback-ветки). +- `docs/work-items/ORCH-046/06-adr/ADR-001-*.md` — ADR (правка ядра). +- `docs/architecture/README.md` / `internals.md` — описание нового хелпера и + поведения заворотов (если reviewer сочтёт необходимым; компонент описать в + разделе Stage Engine / Откаты). +- `CHANGELOG.md` — запись о ORCH-046. + +## 8. Контроль качества / проверка + +```bash +python -m pytest tests/ -q # в контейнере; все тесты зелёные +``` + +Обязательно: стадия `deploy-staging` (8501) перед прод-деплоем (self-hosting). diff --git a/docs/work-items/ORCH-046/03-acceptance-criteria.md b/docs/work-items/ORCH-046/03-acceptance-criteria.md new file mode 100644 index 0000000..4882b4d --- /dev/null +++ b/docs/work-items/ORCH-046/03-acceptance-criteria.md @@ -0,0 +1,99 @@ +# Критерии приёмки — ORCH-046 + +Work Item ID: ORCH-046 +Stage: analysis +Author: analyst +Date: 2026-06-06 + +Каждый критерий имеет чёткое условие PASS/FAIL. Reviewer/Tester проверяют по +этому списку. + +## AC-1 — Дословные P0/P1 findings ревьюера в task_desc + +**Условие:** при reviewer REQUEST_CHANGES (откат `review`/`testing` → +`development`) строка `task_desc`, переданная в `enqueue_job("developer", …)`, +содержит ДОСЛОВНЫЙ текст findings уровня P0/P1 из `12-review.md` (не только +ссылку). + +- **PASS:** в `task_desc` присутствуют дословные строки P0/P1 пунктов из секции + `## Findings` файла `12-review.md`. +- **FAIL:** `task_desc` содержит только ссылку на файл, без текста findings (при + наличии валидного файла с P0/P1). + +## AC-2 — Причина падения тестера в task_desc + +**Условие:** при tester FAIL (`check_tests_passed`, откат `testing` → +`development`) строка `task_desc` содержит причину падения: `reason` из гейта + +релевантный фрагмент тела `13-test-report.md`. + +- **PASS:** `task_desc` содержит `reason` И непустой фрагмент тела отчёта + (вывод pytest / FAIL-строки / Итог), когда отчёт валиден. +- **FAIL:** `task_desc` содержит только ссылку на файл без причины/фрагмента + (при наличии валидного отчёта). + +## AC-3 — Ссылка на полный файл сохранена + +**Условие:** в обеих ветках (reviewer, tester) `task_desc` по-прежнему содержит +ссылку на полный файл-артефакт (`docs/work-items//12-review.md` / +`13-test-report.md`) как дополнительный контекст. + +- **PASS:** путь к файлу присутствует в `task_desc` в обоих сценариях. +- **FAIL:** ссылка на файл удалена/отсутствует. + +## AC-4 — Парсер устойчив к отсутствию/битому файлу (graceful) + +**Условие:** `extract_review_findings(path)` и `extract_test_failures(path)` +НИКОГДА не бросают исключение; при отсутствующем/нечитаемом/битом файле +возвращают `""`, а вызывающий код в `stage_engine` делает fallback на прежнюю +ссылку-строку. + +- **PASS:** на несуществующем пути, пустом файле, файле без секций, битом + markdown/YAML — функции возвращают `""` без исключения; `advance_stage` + отрабатывает откат как раньше (ссылка-строка в `task_desc`). +- **FAIL:** любое исключение наружу из парсера или из `advance_stage` из-за + парсинга. + +## AC-5 — Тесты зелёные + новые юнит-тесты парсера + +**Условие:** существующие тесты не сломаны; добавлены юнит-тесты парсера, +покрывающие: findings есть / findings пусто / битый YAML(frontmatter) / только +P3 (нет P0/P1). + +- **PASS:** `python -m pytest tests/ -q` зелёный; `tests/test_review_parse.py` + содержит как минимум кейсы: P0/P1 присутствуют → текст возвращён; нет + findings/только P2-P3 → `""`; битый файл → `""`; отсутствующий путь → `""`; + для test-report: FAIL-фрагмент извлечён / пустой отчёт → `""`. +- **FAIL:** падение существующих тестов или отсутствие перечисленных кейсов. + +## AC-6 — Retry-счётчик и rollback НЕ изменены по поведению + +**Условие:** логика `_developer_retry_count`, `MAX_DEVELOPER_RETRIES = 3`, +последовательность откатов и поля `AdvanceResult` (`rolled_back_to`, +`enqueued_agent`, `enqueued_job_id`, `alerted`) идентичны прежним. + +- **PASS:** существующие тесты `test_stage_engine.py` на rollback/retry зелёные; + при 4-м заходе по-прежнему alert вместо enqueue; меняется только текст + `task_desc`. +- **FAIL:** изменилось число retry, порядок вызовов, или значения полей + `AdvanceResult`. + +## AC-7 — Out-of-scope не затронут + +**Условие:** не изменены: `check_*` гейты, реестр `QG_CHECKS`, сигнатуры +публичных функций (`advance_stage`, `_run_qg`, `check_*`), webhook-пути, формат +Plane-комментариев. + +- **PASS:** `git diff` не содержит изменений в `src/qg/checks.py` (логика + гейтов), сигнатурах публичных функций, `src/webhooks/*`, + `usage.build_status_comment`; `test_qg_registry_snapshot` зелёный. +- **FAIL:** любое из перечисленного изменено. + +## AC-8 — Документация и ADR обновлены (golden source) + +**Условие:** правка ядра → заведён ADR (`06-adr/`), обновлён `CHANGELOG.md`, при +необходимости — `docs/architecture/README.md`/`internals.md` (раздел Stage +Engine / Откаты). + +- **PASS:** присутствует `docs/work-items/ORCH-046/06-adr/ADR-001-*.md`; в + `CHANGELOG.md` есть запись ORCH-046. +- **FAIL:** ADR или запись в CHANGELOG отсутствуют. diff --git a/docs/work-items/ORCH-046/04-test-plan.yaml b/docs/work-items/ORCH-046/04-test-plan.yaml new file mode 100644 index 0000000..9a01980 --- /dev/null +++ b/docs/work-items/ORCH-046/04-test-plan.yaml @@ -0,0 +1,108 @@ +work_item: ORCH-046 +description: > + Тест-план для встраивания дословного текста findings reviewer/tester в + task_desc при заворотах деву. Покрывает новый парсер src/review_parse.py + (graceful, never-raise) и две rollback-ветки src/stage_engine.py. + +tests: + # --- Парсер review findings (extract_review_findings) ------------------- + - id: TC-01 + type: unit + description: "extract_review_findings возвращает дословный текст P0/P1 при их наличии в 12-review.md" + module: tests/test_review_parse.py + covers: [AC-1, AC-5] + expected: PASS + + - id: TC-02 + type: unit + description: "extract_review_findings возвращает '' когда есть только P2/P3 (нет must-fix P0/P1)" + module: tests/test_review_parse.py + covers: [AC-5] + expected: PASS + + - id: TC-03 + type: unit + description: "extract_review_findings возвращает '' для отсутствующего файла (несуществующий путь), без исключения" + module: tests/test_review_parse.py + covers: [AC-4] + expected: PASS + + - id: TC-04 + type: unit + description: "extract_review_findings возвращает '' для битого/пустого файла или markdown без секции ## Findings, без исключения" + module: tests/test_review_parse.py + covers: [AC-4, AC-5] + expected: PASS + + - id: TC-05 + type: unit + description: "extract_review_findings усекает очень длинные findings до лимита с маркером truncated" + module: tests/test_review_parse.py + covers: [AC-1] + expected: PASS + + # --- Парсер test failures (extract_test_failures) ---------------------- + - id: TC-06 + type: unit + description: "extract_test_failures извлекает релевантный фрагмент тела (Вывод pytest / FAIL-строки / Итог) из 13-test-report.md с result: FAIL" + module: tests/test_review_parse.py + covers: [AC-2, AC-5] + expected: PASS + + - id: TC-07 + type: unit + description: "extract_test_failures возвращает '' для отсутствующего файла, без исключения" + module: tests/test_review_parse.py + covers: [AC-4] + expected: PASS + + - id: TC-08 + type: unit + description: "extract_test_failures возвращает '' для битого/пустого отчёта (нет тела/секций), без исключения" + module: tests/test_review_parse.py + covers: [AC-4, AC-5] + expected: PASS + + # --- Интеграция со stage_engine (rollback task_desc) ------------------- + - id: TC-09 + type: integration + description: "advance_stage: reviewer REQUEST_CHANGES -> в enqueue_job('developer') task_desc содержит дословные P0/P1 findings И ссылку на 12-review.md" + module: tests/test_stage_engine.py + covers: [AC-1, AC-3] + expected: PASS + + - id: TC-10 + type: integration + description: "advance_stage: tester check_tests_passed FAIL -> task_desc содержит reason + фрагмент 13-test-report.md И ссылку на файл" + module: tests/test_stage_engine.py + covers: [AC-2, AC-3] + expected: PASS + + - id: TC-11 + type: integration + description: "advance_stage: reviewer REQUEST_CHANGES при отсутствующем/битом 12-review.md -> graceful fallback, task_desc = прежняя ссылка-строка, без исключения" + module: tests/test_stage_engine.py + covers: [AC-4, AC-3] + expected: PASS + + - id: TC-12 + type: integration + description: "advance_stage: rollback/retry поведение неизменно — последовательность откатов, _developer_retry_count, alert на 4-й заход, поля AdvanceResult" + module: tests/test_stage_engine.py + covers: [AC-6] + expected: PASS + + # --- Регресс / неизменность out-of-scope ------------------------------ + - id: TC-13 + type: integration + description: "Реестр QG_CHECKS не изменён (snapshot), гейты check_* нетронуты" + module: tests/test_qg_registry_snapshot.py + covers: [AC-7] + expected: PASS + + - id: TC-14 + type: integration + description: "Полный регресс существующего набора зелёный: python -m pytest tests/ -q" + module: tests/ + covers: [AC-5, AC-6, AC-7] + expected: PASS diff --git a/docs/work-items/ORCH-046/06-adr/ADR-001-embed-findings-in-task-desc.md b/docs/work-items/ORCH-046/06-adr/ADR-001-embed-findings-in-task-desc.md new file mode 100644 index 0000000..b6bca38 --- /dev/null +++ b/docs/work-items/ORCH-046/06-adr/ADR-001-embed-findings-in-task-desc.md @@ -0,0 +1,143 @@ +# ADR-001: дословный текст findings reviewer/tester встраивается в `task_desc` через отдельный graceful-парсер + +- **Статус:** Accepted +- **Дата:** 2026-06-06 +- **Задача:** ORCH-046 +- **Область:** ЯДРО `src/stage_engine.py` (rollback-ветки) + новый модуль `src/review_parse.py`. Общий прод-инстанс (orchestrator + enduro-trails), self-hosting. + +## Контекст + +При заворотах задачи на `development` (откат) `stage_engine` формирует `task_desc`, +который попадает в `.task-dev.md` запускаемого developer-агента. В двух ветках +`_handle_qg_failure_rollbacks` этот текст содержит **только ссылку на файл-артефакт**: + +- reviewer REQUEST_CHANGES (`src/stage_engine.py` ~стр. 419) → `…Fix findings in docs/work-items//12-review.md`; +- tester `check_tests_passed` FAIL (~стр. 455) → `…Fix failures described in docs/work-items//13-test-report.md`. + +Developer-агент получает инструкцию «иди читай файл»; ключевые претензии (P0/P1 +ревьюера, причина падения тестера) теряются — агент повторяет ту же ошибку, и +задача заворачивается снова. Это «испорченный телефон»: расход retry-бюджета +(`MAX_DEVELOPER_RETRIES = 3`), токенов и простой конвейера (для всех проектов +общего инстанса). + +Ограничение из BRD/ТЗ (вариант A, выбран Owner): минимальная, низкорисковая +правка ядра. Любая регрессия в формировании `task_desc` или (тем более) +исключение в `advance_stage` останавливает конвейер ВСЕХ проектов — следовательно +извлечение текста обязано быть полностью graceful. + +## Решение + +Встраивать **дословный текст ключевых замечаний** в `task_desc` при заворотах, +сохраняя ссылку на полный файл как дополнительный контекст. Извлечение вынести в +отдельный defensive-модуль, чтобы изолировать blast radius от ядра. + +1. **Новый модуль `src/review_parse.py`** с двумя чистыми функциями чтения с диска: + - `extract_review_findings(path: str) -> str` — дословные пункты P0/P1 из секции + `## Findings` файла `12-review.md`; + - `extract_test_failures(path: str) -> str` — релевантный фрагмент тела + `13-test-report.md` (приоритет: `## Вывод pytest` → FAIL-строки `## Результаты` + → `## Итог`). + - **Контракт «never raise»** (как `src/frontmatter.py` и + `src/qg/checks.py::_parse_tests_verdict`): любая ошибка — нет файла, IOError, + кривой markdown/YAML, нет секции — возвращает `""`. Логирование на + `logger.debug` (`logging.getLogger("orchestrator.review_parse")`). Никаких + сетевых вызовов. + - Результат усекается модульными лимитами `MAX_FINDINGS_CHARS`, + `MAX_FAILURES_CHARS` (≈2000) с маркером `…(truncated)`; полный контекст всегда + остаётся в файле. + +2. **Две ветки `_handle_qg_failure_rollbacks` в `src/stage_engine.py`** строят путь + через `get_worktree_path(repo, branch)` (как `_check_review_approved_by_branch`), + вызывают соответствующий парсер и: + - если результат непустой — встраивают findings/фрагмент **дословно** под + подзаголовком + оставляют ссылку как «полный контекст»; + - если результат пустой — `task_desc` остаётся **как сейчас** (graceful fallback + на ссылку-строку); + - tester-ветка дополнительно всегда включает `reason` из гейта (он уже доступен). + +3. **Изоляция ядра.** Меняется ТОЛЬКО содержимое строки `task_desc`, передаваемой в + `enqueue_job`. Последовательность отката (`update_task_stage` → + `notify_stage_change` → `plane_notify_stage` → [`set_issue_in_progress` для + тестера] → проверка `_developer_retry_count` < `MAX_DEVELOPER_RETRIES` → + `enqueue_job`/`send_telegram`), значения `AdvanceResult` (`rolled_back_to`, + `enqueued_agent`, `enqueued_job_id`, `alerted`) и Plane-комментарии — без + изменений. + +### Почему отдельный модуль, а не inline в `stage_engine` + +- Тестируемость: парсер покрывается юнит-тестами `tests/test_review_parse.py` + изолированно от тяжёлого `advance_stage`. +- Blast radius: вся парсинг-логика (и её исключения) физически отделена от + hot-path ядра; ядро только подставляет строку и делает try-around-граничный + fallback. +- Согласованность с уже принятым паттерном defensive-парсеров + (`frontmatter.py`). + +### Почему НЕ переиспользуется `frontmatter.read_frontmatter_value` + +Тот хелпер читает одиночное значение из YAML-frontmatter. Здесь нужно извлекать +**тело markdown** (подсекции `## Findings`/`### P0`, фрагменты `## Вывод pytest`), +а не frontmatter-ключ. Это другая задача парсинга; общий контракт «never raise» +повторяется намеренно (как уже зафиксировано в ORCH-016/ADR-001 §5 — слияние +парсеров отдельной задачей). + +### Почему per-work-item ADR, а не глобальный + +Изменение НЕ добавляет гейт/стадию/компонент и НЕ меняет топологию или реестр +`QG_CHECKS` — это обогащение содержимого `task_desc` в существующих rollback-ветках +плюс вспомогательный модуль. По прецеденту ORCH-047/ADR-001 такого класса правки +фиксируются per-work-item ADR. Глобальный `docs/architecture/adr/` не требуется. + +### Альтернативы (отклонены) + +- **Inline-парсинг прямо в `stage_engine`** — отклонено: раздувает ядро, хуже + тестируется, исключения ближе к hot-path. +- **Менять промпты reviewer/tester, чтобы они сами клали суть в `task_desc`** — + отклонено: `task_desc` формирует ядро, а не агент; зависит от дисциплины двух + агентов вместо детерминированного кода; шире поверхность регрессии. +- **Передавать весь файл целиком в `task_desc`** — отклонено: раздувает промпт + developer-агента и стоимость токенов; теряется фокус на must-fix. Усечение по + P0/P1 + лимит решает проблему «испорченного телефона» дешевле. + +## Последствия + +- **Плюс:** developer-агент видит суть претензий (P0/P1, причина FAIL) сразу в + `.task-dev.md`; меньше повторных заворотов, экономия retry-бюджета и токенов на + всех проектах общего инстанса. +- **Плюс:** при битом/отсутствующем артефакте поведение не хуже текущего (ссылка + сохраняется); ссылка на полный файл присутствует всегда (AC-3). +- **Плюс:** изменение аддитивное — публичные сигнатуры (`advance_stage`, `_run_qg`, + `check_*`), реестр `QG_CHECKS`, webhook-пути и `build_status_comment` не + затрагиваются; снапшот `test_qg_registry_snapshot` остаётся зелёным (AC-7). +- **Минус/ограничение:** парсинг тела markdown чувствительнее к формату артефактов, + чем чтение одного frontmatter-ключа. Митигировано: распознавание P0/P1 устойчиво + к регистру/тире; при несовпадении формата — пустой результат и fallback на + ссылку (никогда не исключение). +- **Минус:** усечение лимитом может обрезать длинные findings — приемлемо, полный + контекст остаётся в файле, ссылка сохранена. +- **Self-hosting риск:** правка ядра в общем прод-контейнере. Обязателен прогон + `deploy-staging` (8501) перед прод-деплоем; прод-контейнер `orchestrator` (8500) + не перезапускать в рамках разработки/тестинга. Граничный риск — исключение из + парсера в `advance_stage`; закрыт контрактом «never raise» + юнит-кейсами на + битый/пустой/отсутствующий ввод (AC-4, AC-5). + +## Влияние на документацию (golden source) + +В PR разработки (вместе с кодом) обновить: +- `docs/architecture/README.md` — раздел **Stage Engine** / **Откаты**: упомянуть, + что `task_desc` при заворотах reviewer/tester несёт дословные findings + ссылку, + и новый модуль `src/review_parse.py` (defensive, never-raise). +- `CHANGELOG.md` — запись ORCH-046. +- `docs/architecture/internals.md` — по усмотрению reviewer, если детализируется + поток отката. + +## Связи + +- BRD/ТЗ/AC: `docs/work-items/ORCH-046/01-brd.md`, `02-trz.md`, `03-acceptance-criteria.md`. +- Образцы паттерна «never raise»: `src/frontmatter.py`, + `src/qg/checks.py::_parse_tests_verdict`. +- Каноны артефактов: `.openclaw/agents/reviewer.md` (`12-review.md` `## Findings`), + `.openclaw/agents/tester.md` (`13-test-report.md` `result:` + тело). +- Прецедент per-work-item ADR на правку парсинга: ORCH-047/ADR-001. +- Технические риски: `docs/work-items/ORCH-046/10-tech-risks.md`. +- Staging-страховка: `docs/architecture/adr/adr-0003-staging-gate.md`. diff --git a/docs/work-items/ORCH-046/10-tech-risks.md b/docs/work-items/ORCH-046/10-tech-risks.md new file mode 100644 index 0000000..a232e08 --- /dev/null +++ b/docs/work-items/ORCH-046/10-tech-risks.md @@ -0,0 +1,29 @@ +# Технические риски — ORCH-046 + +Work Item ID: ORCH-046 +Stage: architecture +Author: architect +Date: 2026-06-06 + +Связано: `06-adr/ADR-001-embed-findings-in-task-desc.md`. + +| # | Риск | Вероятн. | Влияние | Митигация | Контроль (AC/тест) | +|---|------|----------|---------|-----------|--------------------| +| R-1 | Исключение из парсера всплывает в `advance_stage` → встаёт конвейер ВСЕХ проектов (self-hosting, общий инстанс) | Низк. | **Критич.** | Контракт «never raise» в `review_parse.py`; вызов в `stage_engine` обёрнут так, что пустой результат → fallback на прежнюю ссылку-строку | AC-4; юнит-кейсы «нет файла / битый YAML / пустой / только P3» в `tests/test_review_parse.py` | +| R-2 | Регрессия в последовательности отката или полях `AdvanceResult` (меняется не только `task_desc`) | Низк. | Высок. | Жёсткий инвариант ТЗ §3.3: трогать ТОЛЬКО строку `task_desc`; порядок вызовов и условия retry неизменны | AC-6; существующие `tests/test_stage_engine.py` (rollback/retry) зелёные | +| R-3 | Парсер чувствителен к формату артефактов: дрейф `12-review.md`/`13-test-report.md` → пустой результат | Сред. | Низк. | Распознавание P0/P1 устойчиво к регистру/тире; при несовпадении → `""` + fallback на ссылку (деградация, не отказ) | AC-1/AC-2/AC-4 | +| R-4 | Раздувание `task_desc` длинными findings → рост стоимости/потеря фокуса developer-агента | Сред. | Низк. | Лимиты `MAX_FINDINGS_CHARS`/`MAX_FAILURES_CHARS` (~2000) + маркер `…(truncated)`; only P0/P1 (P2/P3 отброшены) | AC-1; проверка усечения в юнит-тестах | +| R-5 | Случайный выход за out-of-scope (правка `check_*`, `QG_CHECKS`, сигнатур, webhooks, `build_status_comment`) | Низк. | Сред. | Явный out-of-scope в ТЗ §4/§6; ревью diff | AC-7; `test_qg_registry_snapshot` зелёный | +| R-6 | Прод-деплой self без страховки staging | Низк. | **Критич.** | Обязательная стадия `deploy-staging` (8501); прод `orchestrator` (8500) не рестартить в разработке/тестинге | adr-0003; стадийный гейт `check_staging_status` | +| R-7 | Дублирование defensive-парсинга (3-й модуль рядом с `frontmatter.py` и `_parse_tests_verdict`) → техдолг | Сред. | Низк. | Осознанно принято (как ORCH-016/ADR-001 §5): малый blast radius важнее DRY; слияние парсеров — отдельная follow-up задача | — (техдолг, не блокер) | + +## Заметки + +- **Граничный try в ядре.** Даже при контракте «never raise» в `review_parse`, + вызов в `stage_engine` следует считать недоверенным: подстановка результата в + `task_desc` не должна зависеть от внутренней корректности парсера. Fallback на + ссылку-строку обязателен и при пустом результате, и при любой неожиданности. +- **Эскалация не требуется.** Изменение укладывается в принципы (минимум + зависимостей, raw-парсинг без новых либ, без новых компонентов/стадий/QG). + Лейбл `arch:major-change` НЕ ставится; возврат в Анализ не требуется — ТЗ + реализуемо без нарушения принципов. diff --git a/docs/work-items/ORCH-046/12-review.md b/docs/work-items/ORCH-046/12-review.md new file mode 100644 index 0000000..d169abb --- /dev/null +++ b/docs/work-items/ORCH-046/12-review.md @@ -0,0 +1,83 @@ +--- +type: review +work_item_id: ORCH-046 +verdict: APPROVED +version: 1 +--- + +# Review ORCH-046 + +## Summary + +Правка ядра «вариант A»: при заворотах на `development` `task_desc` теперь несёт +**дословный must-fix текст** (P0/P1 ревьюера, причина FAIL тестера) вместо одной +ссылки на файл. Извлечение вынесено в новый defensive-модуль `src/review_parse.py` +с контрактом «never raise»; две rollback-ветки `_handle_qg_failure_rollbacks` +встраивают текст и сохраняют ссылку как «Полный контекст», при пустом/битом +артефакте — graceful-фоллбэк на прежнюю строку. + +Реализация полностью соответствует ТЗ (`02-trz.md`), ADR-001 и всем критериям +приёмки. Документация обновлена в этом же PR. Тесты зелёные (`461 passed`). + +Проверено по осям: + +**1. Соответствие ТЗ.** Сигнатуры `extract_review_findings`/`extract_test_failures` +точно как в ТЗ §2; never-raise, логирование на `logger.debug`, модульные лимиты +`MAX_FINDINGS_CHARS`/`MAX_FAILURES_CHARS`, отбрасывание frontmatter, устойчивость +P0/P1-заголовков к регистру/тире, пропуск плейсхолдеров `(если есть)`/`<…>`, +приоритет источников тестера (`## Вывод pytest` → FAIL-строки `## Результаты` → +`## Итог`). Префикс `task_desc`, `reason` в tester-ветке, ссылка-фоллбэк — как +предписано §3. API/БД/QG не тронуты (§4–6). + +**2. Соответствие ADR-001.** Отдельный модуль (blast radius), путь через +`get_worktree_path`, изоляция ядра (меняется только строка `task_desc`), +последовательность отката и поля `AdvanceResult` сохранены. Per-work-item ADR +обоснован. Реализация ⇄ решение совпадают. + +**3. Качество кода.** Docstrings на всех публичных функциях; defensive `_read` +ловит `OSError`, внешний `try/except Exception` в обоих экстракторах гарантирует +never-raise (подтверждено кейсом на directory-path). Регэксп `_P01_HEADER_RE` +корректно отсекает ложные совпадения (`P05` и т.п.). Код читабелен, без дублей. + +**4. Качество тестов.** `tests/test_review_parse.py` покрывает TC-01..08 (findings +есть / только P2-P3 / нет файла / битый YAML / усечение / регистр-тире / directory). +`tests/test_stage_engine.py::TestRollbackTaskDescEmbedding` проверяет встраивание +в обе ветки, graceful-фоллбэк, неизменность retry/rollback на 4-м заходе (alert +вместо enqueue). Содержательные, не тривиальные. + +## Findings + +### P0 — Blocker +- [ ] (нет) + +### P1 — Must fix +- [ ] (нет) + +### P2 — Should fix +- [ ] (нет) + +## Соответствие критериям приёмки + +- AC-1 (дословные P0/P1 в `task_desc`) — PASS: `Findings (P0/P1):\n{findings}`. +- AC-2 (причина тестера: `reason` + фрагмент тела) — PASS: `Причина: {reason}` + `Детали:`. +- AC-3 (ссылка на полный файл сохранена) — PASS: «Полный контекст»/fallback-ссылка в обеих ветках. +- AC-4 (graceful never-raise) — PASS: `""`→ссылка-фоллбэк, исключений нет (тесты TC-03/04/07/08, directory-path). +- AC-5 (тесты зелёные + новые юнит-тесты) — PASS: `461 passed`; все перечисленные кейсы присутствуют. +- AC-6 (retry/rollback не изменены) — PASS: TC-12 + существующие rollback-тесты зелёные. +- AC-7 (out-of-scope не затронут) — PASS: diff не касается `src/qg/checks.py`, `src/webhooks/*`, `usage.py`, `stages.py`, `main.py`; сигнатуры публичных функций не менялись. +- AC-8 (документация + ADR) — PASS: ADR-001 заведён, `CHANGELOG.md` и `docs/architecture/README.md` обновлены. + +## Документация + +Обновлена корректно и в том же PR (golden source соблюдён): +- `docs/work-items/ORCH-046/06-adr/ADR-001-embed-findings-in-task-desc.md` — заведён (правка ядра). +- `CHANGELOG.md` — запись ORCH-046 в `[Unreleased] / Added`. +- `docs/architecture/README.md` — добавлен компонент **Review/Test Parsers** и раздел **Обогащение `task_desc` при заворотах (ORCH-046)**. + +Изменение `src/` сопровождено обновлением документации — требование п.4/п.6 правил +агентов выполнено. + +## Примечание (self-hosting) +Правка ядра в общем прод-инстансе. Перед прод-деплоем обязательна стадия +`deploy-staging` (8501) согласно ADR-001 / CLAUDE.md — это страховка следующих +стадий, не блокер ревью. diff --git a/docs/work-items/ORCH-046/13-test-report.md b/docs/work-items/ORCH-046/13-test-report.md new file mode 100644 index 0000000..811fc85 --- /dev/null +++ b/docs/work-items/ORCH-046/13-test-report.md @@ -0,0 +1,92 @@ +--- +type: test-report +work_item_id: ORCH-046 +result: PASS +--- + +# Test Report — ORCH-046 + +Встраивание дословного must-fix текста findings reviewer/tester в `task_desc` +при заворотах на `development` (новый модуль `src/review_parse.py` + две +rollback-ветки `src/stage_engine.py`). + +## Окружение +- Python: 3.12.13 +- pytest: 8.3.3 (asyncio mode=AUTO) +- Ветка: feature/ORCH-046-stage-engine-pass-reviewer-tes +- Дата: 2026-06-06 + +## Результаты + +| TC ID | Описание | Покрывает | Результат | +|-------|----------|-----------|-----------| +| TC-01 | `extract_review_findings` возвращает дословный P0/P1 текст | AC-1, AC-5 | PASS | +| TC-02 | `extract_review_findings` → `""` при только P2/P3 | AC-5 | PASS | +| TC-03 | `extract_review_findings` → `""` для отсутствующего файла | AC-4 | PASS | +| TC-04 | `extract_review_findings` → `""` для битого/без секции файла | AC-4, AC-5 | PASS | +| TC-05 | `extract_review_findings` усекает длинный текст с маркером truncated | AC-1 | PASS | +| TC-06 | `extract_test_failures` извлекает фрагмент тела (Вывод pytest/FAIL/Итог) | AC-2, AC-5 | PASS | +| TC-07 | `extract_test_failures` → `""` для отсутствующего файла | AC-4 | PASS | +| TC-08 | `extract_test_failures` → `""` для битого/пустого отчёта | AC-4, AC-5 | PASS | +| TC-09 | reviewer REQUEST_CHANGES → `task_desc` содержит P0/P1 + ссылку | AC-1, AC-3 | PASS | +| TC-10 | tester FAIL → `task_desc` содержит reason + фрагмент + ссылку | AC-2, AC-3 | PASS | +| TC-11 | graceful fallback при отсутствующем/битом файле (обе ветки) | AC-4, AC-3 | PASS | +| TC-12 | rollback/retry поведение неизменно (alert на 4-й заход, поля AdvanceResult) | AC-6 | PASS | +| TC-13 | Реестр `QG_CHECKS` не изменён (snapshot), гейты нетронуты | AC-7 | PASS | +| TC-14 | Полный регресс существующего набора зелёный | AC-5, AC-6, AC-7 | PASS | + +Сопоставление TC ↔ тесты: +- TC-01..08 → `tests/test_review_parse.py` (`TestExtractReviewFindings`, `TestExtractTestFailures`), 14 кейсов, все PASS. +- TC-09..12 → `tests/test_stage_engine.py::TestRollbackTaskDescEmbedding`, все PASS. +- TC-13 → `tests/test_qg_registry_snapshot.py` (registry/callables/transitions snapshot), все PASS. +- TC-14 → полный прогон `pytest tests/` → **461 passed**. + +## Smoke test API (read-only, прод-инстанс не затронут) + +| Endpoint | HTTP | Ответ | +|----------|------|-------| +| GET /health | 200 | `{"status":"ok","service":"orchestrator"}` | +| GET /status | 200 | active_tasks включает task 37 (ORCH-046, stage=testing) | +| GET /queue | 200 | counts: queued=0, running=1, failed=0; breaker=closed; preflight_ok=true | + +> `curl` в окружении отсутствует — smoke выполнен через `urllib`. Только GET-запросы, +> деструктивных операций над прод-контейнером не выполнялось (self-hosting safety). + +## Вывод pytest + +``` +============================= test session starts ============================== +platform linux -- Python 3.12.13, pytest-8.3.3, pluggy-1.6.0 +rootdir: .../feature_ORCH-046-stage-engine-pass-reviewer-tes +configfile: pytest.ini +plugins: anyio-4.13.0, asyncio-0.23.8 +asyncio: mode=Mode.AUTO +... +======================== 461 passed, 1 warning in 7.59s ======================== +``` + +Прицельный прогон ORCH-046 (`test_review_parse.py` + `test_stage_engine.py` + +`test_qg_registry_snapshot.py`): **53 passed**. + +Единственный warning — преэкзистентный `PydanticDeprecatedSince20` в `src/config.py` +(не связан с ORCH-046). + +## Покрытие критериев приёмки + +| AC | Критерий | Подтверждение | Статус | +|----|----------|---------------|--------| +| AC-1 | Дословные P0/P1 в `task_desc` | TC-01, TC-09 | PASS | +| AC-2 | Причина тестера (reason + фрагмент) в `task_desc` | TC-06, TC-10 | PASS | +| AC-3 | Ссылка на полный файл сохранена | TC-09, TC-10, TC-11 | PASS | +| AC-4 | Парсер graceful (never-raise) | TC-03, TC-04, TC-07, TC-08, TC-11 | PASS | +| AC-5 | Тесты зелёные + новые юнит-тесты | TC-14 (461 passed) | PASS | +| AC-6 | Retry/rollback не изменены | TC-12 | PASS | +| AC-7 | Out-of-scope не затронут | TC-13 | PASS | +| AC-8 | Документация + ADR | проверено reviewer (12-review.md, APPROVED) | PASS | + +## Итог + +**PASS** — все 14 TC из тест-плана зелёные, полный регресс 461 passed, +smoke API 200 по всем эндпоинтам, прод-инстанс здоров. Все критерии приёмки +выполнены. Задача готова к стадии `deploy-staging` (8501) — обязательной +страховке self-hosting перед прод-деплоем. diff --git a/src/review_parse.py b/src/review_parse.py new file mode 100644 index 0000000..db57778 --- /dev/null +++ b/src/review_parse.py @@ -0,0 +1,205 @@ +"""Defensive extractors for reviewer / tester artifact bodies (ORCH-046). + +When a task is rolled back to ``development`` the stage engine builds the +``task_desc`` that ends up in the developer agent's ``.task-dev.md``. Historically +that text only carried a *link* to the artifact file (12-review.md / +13-test-report.md); the developer agent had to go read the file, and the key +must-fix points (reviewer P0/P1 findings, tester failure reason) were lost in +transit — "испорченный телефон" that burns the retry budget. + +This module extracts the **verbatim** must-fix text so the stage engine can embed +it directly in ``task_desc`` (ADR docs/work-items/ORCH-046/06-adr/ADR-001-*). + +Contract — **never raises** (mirrors ``src/frontmatter.py`` and +``src/qg/checks.py::_parse_tests_verdict``): any error — missing file, IOError, +malformed markdown/YAML, missing section — yields ``""``. The caller then falls +back to the previous link-only ``task_desc``. No network calls; disk reads only. +""" + +import logging +import re + +logger = logging.getLogger("orchestrator.review_parse") + +# Truncation limits (module-level per ТЗ §2.3). The full context always stays in +# the artifact file; the embedded text is a focused excerpt. +MAX_FINDINGS_CHARS = 2000 +MAX_FAILURES_CHARS = 2000 + +_TRUNCATED_MARKER = "\n…(truncated)" + +# Recognize a `### P0`/`### P1` subsection header by the presence of the P0/P1 +# token, tolerant to case and the dash/em-dash that follows it. +_P01_HEADER_RE = re.compile(r"(? str | None: + """Read a file as UTF-8. Never raises; returns None on any OS error.""" + try: + with open(path, "r", encoding="utf-8", errors="replace") as f: + return f.read() + except OSError as e: + logger.debug(f"review_parse: cannot open {path}: {e}") + return None + + +def _strip_frontmatter(content: str) -> str: + """Drop a leading ``--- … ---`` YAML frontmatter block, if present.""" + if content.startswith("---"): + parts = content.split("---", 2) + if len(parts) >= 3: + return parts[2] + return content + + +def _truncate(text: str, limit: int) -> str: + """Trim ``text`` to ``limit`` chars, appending a truncation marker if cut.""" + if len(text) <= limit: + return text + return text[:limit].rstrip() + _TRUNCATED_MARKER + + +def _section_body(md: str, heading_token: str) -> str: + """Return the body lines under the first ``## <…heading_token…>`` heading. + + Capture stops at the next level-2 (``## ``) heading. Matching is + case-insensitive substring match on the heading line, so callers pass a token + like ``"Вывод pytest"`` or ``"Findings"``. ``### ``-level headers do NOT + delimit the section (they start with ``"### "``, not ``"## "``). + """ + out: list[str] = [] + capturing = False + for line in md.splitlines(): + if line.startswith("## "): + if capturing: + break + if heading_token.lower() in line.lower(): + capturing = True + continue + if capturing: + out.append(line) + return "\n".join(out) + + +def _is_placeholder_item(text: str) -> bool: + """True for empty or template-placeholder list items (non-substantive). + + The canonical reviewer template seeds each severity with + ``- [ ] <описание> (если есть)``. Such lines must be ignored so an empty P0/P1 + subsection does not leak the placeholder into ``task_desc``. + """ + t = text.strip() + if not t: + return True + if "(если есть)" in t: + return True + # An item whose entire payload is an angle-bracket placeholder, e.g. "<описание>". + if t.startswith("<") and t.endswith(">"): + return True + return False + + +def _item_payload(line: str) -> str | None: + """If ``line`` is a markdown list item, return its payload text; else None. + + Handles ``- foo``, ``* foo`` and checkbox forms ``- [ ] foo`` / ``- [x] foo``. + """ + m = re.match(r"\s*[-*]\s+(?:\[[ xX]?\]\s*)?(.*)$", line) + if not m: + return None + return m.group(1) + + +def _findings_subsections(findings_body: str): + """Yield ``(header_line, body_lines)`` for each ``### `` subsection.""" + header: str | None = None + body: list[str] = [] + for line in findings_body.splitlines(): + if line.startswith("### "): + if header is not None: + yield header, body + header = line + body = [] + elif header is not None: + body.append(line) + if header is not None: + yield header, body + + +def extract_review_findings(path: str) -> str: + """Дословный текст P0/P1 findings из 12-review.md. Never raises; '' при ошибке/пусто. + + Reads the ``## Findings`` section of a reviewer report and returns the verbatim + P0 (Blocker) and P1 (Must fix) subsection items, suitable for embedding in a + rollback ``task_desc``. P2/P3 are ignored. Empty/placeholder-only subsections + are skipped; if no substantive P0/P1 item exists, returns ``""``. The result is + truncated to ``MAX_FINDINGS_CHARS``. + """ + content = _read(path) + if content is None: + return "" + + try: + body = _strip_frontmatter(content) + findings_body = _section_body(body, "Findings") + if not findings_body.strip(): + return "" + + blocks: list[str] = [] + for header, sub_body in _findings_subsections(findings_body): + if not _P01_HEADER_RE.search(header): + continue + kept: list[str] = [] + for line in sub_body: + payload = _item_payload(line) + if payload is None: + continue + if _is_placeholder_item(payload): + continue + kept.append(line.rstrip()) + if kept: + blocks.append("\n".join([header.rstrip(), *kept])) + + if not blocks: + return "" + return _truncate("\n\n".join(blocks), MAX_FINDINGS_CHARS) + except Exception as e: # defensive: never raise out of the extractor + logger.debug(f"review_parse: extract_review_findings failed for {path}: {e}") + return "" + + +def extract_test_failures(path: str) -> str: + """Релевантный фрагмент тела 13-test-report.md (причина FAIL). Never raises; '' при ошибке/пусто. + + Picks the first non-empty source, in priority order: + 1. ``## Вывод pytest`` — the pytest run output (shows failing tests); + 2. rows of the ``## Результаты`` table that contain ``FAIL``; + 3. ``## Итог`` — the verdict summary. + The result is truncated to ``MAX_FAILURES_CHARS``. The gate ``reason`` is added + by the caller; this returns the report-body excerpt on top of it. + """ + content = _read(path) + if content is None: + return "" + + try: + # 1. pytest output. + pytest_out = _section_body(content, "Вывод pytest").strip() + if pytest_out: + return _truncate(pytest_out, MAX_FAILURES_CHARS) + + # 2. FAIL rows from the results table. + results = _section_body(content, "Результаты") + fail_rows = [ln.rstrip() for ln in results.splitlines() if "FAIL" in ln.upper()] + if fail_rows: + return _truncate("\n".join(fail_rows).strip(), MAX_FAILURES_CHARS) + + # 3. Verdict summary. + itog = _section_body(content, "Итог").strip() + if itog: + return _truncate(itog, MAX_FAILURES_CHARS) + + return "" + except Exception as e: # defensive: never raise out of the extractor + logger.debug(f"review_parse: extract_test_failures failed for {path}: {e}") + return "" diff --git a/src/stage_engine.py b/src/stage_engine.py index 9d9ce13..008790b 100644 --- a/src/stage_engine.py +++ b/src/stage_engine.py @@ -32,6 +32,7 @@ from dataclasses import dataclass, field from .db import get_db, update_task_stage, enqueue_job from .stages import get_next_stage, get_qg_for_stage, get_agent_for_stage from .git_worktree import get_worktree_path +from .review_parse import extract_review_findings, extract_test_failures from .qg.checks import QG_CHECKS from .notifications import ( notify_stage_change, @@ -416,12 +417,24 @@ def _handle_qg_failure_rollbacks( result.rolled_back_to = "development" retry_count = _developer_retry_count(task_id) if retry_count < MAX_DEVELOPER_RETRIES: - task_desc = ( + # ORCH-046: embed the verbatim P0/P1 findings into task_desc so the + # developer agent sees the must-fix points directly (not just a link). + # extract_review_findings never raises; "" -> graceful link-only fallback. + review_ref = f"docs/work-items/{work_item_id}/12-review.md" + review_path = os.path.join(get_worktree_path(repo, branch), review_ref) + findings = extract_review_findings(review_path) + head = ( f"Work item: {work_item_id}\nRepo: {repo}\nBranch: {branch}\n" f"Stage: development\nNote: REQUEST_CHANGES from reviewer " - f"(attempt {retry_count+1}/3). Fix findings in " - f"docs/work-items/{work_item_id}/12-review.md" + f"(attempt {retry_count+1}/3)." ) + if findings: + task_desc = ( + f"{head}\nFindings (P0/P1):\n{findings}\n" + f"Полный контекст: {review_ref}" + ) + else: + task_desc = f"{head} Fix findings in {review_ref}" new_job = enqueue_job("developer", repo, task_desc, task_id=task_id) result.enqueued_agent = "developer" result.enqueued_job_id = new_job @@ -452,11 +465,23 @@ def _handle_qg_failure_rollbacks( ) retry_count = _developer_retry_count(task_id) if retry_count < MAX_DEVELOPER_RETRIES: - task_desc = ( + # ORCH-046: embed the gate `reason` plus a verbatim excerpt of the + # test-report body (pytest output / FAIL rows / Итог) into task_desc. + # extract_test_failures never raises; "" -> graceful reason+link fallback. + report_ref = f"docs/work-items/{work_item_id}/13-test-report.md" + report_path = os.path.join(get_worktree_path(repo, branch), report_ref) + failures = extract_test_failures(report_path) + head = ( f"Work item: {work_item_id}\nRepo: {repo}\nBranch: {branch}\n" - f"Stage: development\nNote: Tests FAILED. " - f"Fix failures described in docs/work-items/{work_item_id}/13-test-report.md" + f"Stage: development\nNote: Tests FAILED. Причина: {reason}." ) + if failures: + task_desc = ( + f"{head}\nДетали:\n{failures}\n" + f"Полный контекст: {report_ref}" + ) + else: + task_desc = f"{head} Fix failures described in {report_ref}" new_job = enqueue_job("developer", repo, task_desc, task_id=task_id) result.enqueued_agent = "developer" result.enqueued_job_id = new_job diff --git a/tests/test_review_parse.py b/tests/test_review_parse.py new file mode 100644 index 0000000..bd8b49c --- /dev/null +++ b/tests/test_review_parse.py @@ -0,0 +1,237 @@ +"""Unit tests for src/review_parse (ORCH-046). + +Covers the defensive extractors that pull verbatim must-fix text out of the +reviewer / tester artifacts for embedding into the rollback ``task_desc``: + + - extract_review_findings (12-review.md, ## Findings -> P0/P1) + - extract_test_failures (13-test-report.md, pytest/FAIL/Итог excerpt) + +Both must NEVER raise (return "" on missing/broken/empty input) and must ignore +template placeholders / non-must-fix severities. See 04-test-plan.yaml (TC-01..08). +""" + +import os +import tempfile + +import pytest + +from src.review_parse import ( + extract_review_findings, + extract_test_failures, + MAX_FINDINGS_CHARS, + MAX_FAILURES_CHARS, +) + + +@pytest.fixture +def write_file(tmp_path): + def _w(name: str, content: str) -> str: + p = tmp_path / name + p.write_text(content, encoding="utf-8") + return str(p) + return _w + + +# --------------------------------------------------------------------------- +# extract_review_findings +# --------------------------------------------------------------------------- +_REVIEW_WITH_FINDINGS = """--- +type: review +work_item_id: ORCH-046 +verdict: REQUEST_CHANGES +version: 1 +--- + +# Review ORCH-046 + +## Summary +Несколько проблем. + +## Findings + +### P0 — Blocker +- [ ] Документация не обновлена при изменении src/review_parse.py + +### P1 — Must fix +- [ ] extract_test_failures не обрабатывает пустой отчёт +- [ ] Отсутствует docstring у _section_body + +### P2 — Should fix +- [ ] Переименовать переменную blocks в more descriptive + +## Документация +Требует обновления README. +""" + + +class TestExtractReviewFindings: + def test_tc01_returns_verbatim_p0_p1(self, write_file): + """TC-01: P0/P1 findings present -> verbatim text returned (AC-1, AC-5).""" + path = write_file("12-review.md", _REVIEW_WITH_FINDINGS) + out = extract_review_findings(path) + # P0 + P1 verbatim items present. + assert "Документация не обновлена при изменении src/review_parse.py" in out + assert "extract_test_failures не обрабатывает пустой отчёт" in out + assert "Отсутствует docstring у _section_body" in out + # Subsection headers preserved. + assert "P0" in out and "P1" in out + # P2 must NOT leak in. + assert "Переименовать переменную" not in out + + def test_tc02_only_p2_p3_returns_empty(self, write_file): + """TC-02: only P2/P3 (no must-fix P0/P1) -> '' (AC-5).""" + content = """--- +verdict: REQUEST_CHANGES +--- + +## Findings + +### P0 — Blocker +- [ ] <описание> (если есть) + +### P1 — Must fix +- [ ] <описание> (если есть) + +### P2 — Should fix +- [ ] Косметика в naming +""" + path = write_file("12-review.md", content) + assert extract_review_findings(path) == "" + + def test_tc03_missing_file_returns_empty(self): + """TC-03: non-existent path -> '' without raising (AC-4).""" + missing = os.path.join(tempfile.gettempdir(), "no-such-review-orch046.md") + assert extract_review_findings(missing) == "" + + def test_tc04_broken_or_no_findings_section_returns_empty(self, write_file): + """TC-04: empty file / markdown without ## Findings -> '' (AC-4, AC-5).""" + # Empty file. + assert extract_review_findings(write_file("empty.md", "")) == "" + # No Findings section. + no_section = "# Review\n\n## Summary\nвсё хорошо\n" + assert extract_review_findings(write_file("nofind.md", no_section)) == "" + # Broken YAML frontmatter (unterminated) — body parsing still graceful. + broken = "---\nverdict: [unclosed\n# Review\nno findings here\n" + assert extract_review_findings(write_file("broken.md", broken)) == "" + + def test_tc05_long_findings_truncated(self, write_file): + """TC-05: very long findings truncated to limit with marker (AC-1).""" + big_item = "- [ ] " + ("x" * 5000) + content = f"## Findings\n\n### P0 — Blocker\n{big_item}\n" + path = write_file("12-review.md", content) + out = extract_review_findings(path) + assert len(out) <= MAX_FINDINGS_CHARS + len("\n…(truncated)") + assert "…(truncated)" in out + + def test_case_insensitive_and_dash_tolerant_header(self, write_file): + """P0/P1 recognized regardless of case / dash style.""" + content = """## Findings + +### p0 - blocker +- [ ] Нижний регистр заголовка + +### P1 — Must fix +- [ ] Em-dash заголовок +""" + out = extract_review_findings(write_file("12-review.md", content)) + assert "Нижний регистр заголовка" in out + assert "Em-dash заголовок" in out + + def test_never_raises_on_directory_path(self, tmp_path): + """Passing a directory path must not raise -> ''.""" + assert extract_review_findings(str(tmp_path)) == "" + + +# --------------------------------------------------------------------------- +# extract_test_failures +# --------------------------------------------------------------------------- +_REPORT_FAIL = """--- +type: test-report +work_item_id: ORCH-046 +result: FAIL +--- + +# Test Report — ORCH-046 + +## Окружение +- Python: 3.12 + +## Результаты + +| TC ID | Описание | Результат | +|-------|----------|-----------| +| TC-01 | парсер findings | PASS | +| TC-09 | rollback task_desc | FAIL | + +## Вывод pytest +FAILED tests/test_stage_engine.py::TestReviewerRequestChanges::test_embed - AssertionError +1 failed, 40 passed in 2.13s + +## Итог +FAIL +""" + + +class TestExtractTestFailures: + def test_tc06_extracts_pytest_output(self, write_file): + """TC-06: relevant body excerpt (pytest output) from FAIL report (AC-2, AC-5).""" + path = write_file("13-test-report.md", _REPORT_FAIL) + out = extract_test_failures(path) + assert "FAILED tests/test_stage_engine.py" in out + assert "1 failed, 40 passed" in out + + def test_priority_falls_back_to_fail_rows(self, write_file): + """No pytest section -> FAIL rows of the results table are used.""" + content = """--- +result: FAIL +--- + +## Результаты + +| TC ID | Описание | Результат | +|-------|----------|-----------| +| TC-01 | ok | PASS | +| TC-09 | broken | FAIL | + +## Итог +FAIL +""" + out = extract_test_failures(write_file("13-test-report.md", content)) + assert "TC-09" in out + assert "broken" in out + # PASS rows are not failure-relevant. + assert "TC-01" not in out + + def test_priority_falls_back_to_itog(self, write_file): + """No pytest section and no FAIL rows -> Итог summary is used.""" + content = """--- +result: FAIL +--- + +## Итог +Регресс упал: смотрите CI лог. +""" + out = extract_test_failures(write_file("13-test-report.md", content)) + assert "Регресс упал" in out + + def test_tc07_missing_file_returns_empty(self): + """TC-07: non-existent path -> '' without raising (AC-4).""" + missing = os.path.join(tempfile.gettempdir(), "no-such-report-orch046.md") + assert extract_test_failures(missing) == "" + + def test_tc08_broken_or_empty_report_returns_empty(self, write_file): + """TC-08: empty / section-less report -> '' without raising (AC-4, AC-5).""" + assert extract_test_failures(write_file("empty.md", "")) == "" + no_sections = "---\nresult: FAIL\n---\n\n# Test Report\nничего полезного\n" + assert extract_test_failures(write_file("nosec.md", no_sections)) == "" + + def test_long_failures_truncated(self, write_file): + """Long pytest output is truncated to the limit with a marker.""" + big = "x" * 5000 + content = f"## Вывод pytest\n{big}\n" + out = extract_test_failures(write_file("13-test-report.md", content)) + assert len(out) <= MAX_FAILURES_CHARS + len("\n…(truncated)") + assert "…(truncated)" in out + + def test_never_raises_on_directory_path(self, tmp_path): + assert extract_test_failures(str(tmp_path)) == "" diff --git a/tests/test_stage_engine.py b/tests/test_stage_engine.py index b07720a..03378d9 100644 --- a/tests/test_stage_engine.py +++ b/tests/test_stage_engine.py @@ -101,6 +101,14 @@ def _jobs(): return [dict(r) for r in rows] +def _job_contents(): + """task_content of every enqueued job, oldest first (ORCH-046 task_desc check).""" + conn = get_db() + rows = conn.execute("SELECT task_content FROM jobs ORDER BY id").fetchall() + conn.close() + return [r[0] for r in rows] + + def _add_developer_runs(task_id, n): conn = get_db() for _ in range(n): @@ -335,6 +343,179 @@ class TestTesterFail: assert _jobs() == [] +# --------------------------------------------------------------------------- +# ORCH-046: rollback task_desc carries verbatim reviewer/tester must-fix text +# --------------------------------------------------------------------------- +_REVIEW_MD = """--- +type: review +work_item_id: ET-001 +verdict: REQUEST_CHANGES +version: 1 +--- + +# Review ET-001 + +## Summary +Есть блокеры. + +## Findings + +### P0 — Blocker +- [ ] Гонка в claim_next_job: отсутствует guard в WHERE + +### P1 — Must fix +- [ ] Нет обработки OSError при чтении отчёта + +### P2 — Should fix +- [ ] Переименовать blocks +""" + +_REPORT_MD = """--- +type: test-report +work_item_id: ET-001 +result: FAIL +--- + +# Test Report — ET-001 + +## Результаты + +| TC ID | Описание | Результат | +|-------|----------|-----------| +| TC-09 | rollback | FAIL | + +## Вывод pytest +FAILED tests/test_stage_engine.py::TestTaskDescEmbedding - AssertionError +1 failed, 50 passed in 3.01s + +## Итог +FAIL +""" + + +class TestRollbackTaskDescEmbedding: + """ORCH-046 AC-1/AC-2/AC-3/AC-4: the rollback task_desc embeds verbatim + must-fix text (reviewer P0/P1, tester reason + report excerpt) plus the link. + """ + + def _patch_worktree(self, monkeypatch, tmp_path, work_item_id, filename, body): + """Make get_worktree_path resolve to tmp_path and seed the artifact file.""" + artifact = tmp_path / "docs" / "work-items" / work_item_id + artifact.mkdir(parents=True, exist_ok=True) + (artifact / filename).write_text(body, encoding="utf-8") + monkeypatch.setattr( + stage_engine, "get_worktree_path", lambda repo, branch: str(tmp_path) + ) + + def test_tc09_reviewer_embeds_p0_p1_and_link(self, monkeypatch, tmp_path): + """TC-09: reviewer REQUEST_CHANGES -> task_desc has verbatim P0/P1 + link.""" + monkeypatch.setattr( + stage_engine, "QG_CHECKS", + {**stage_engine.QG_CHECKS, + "check_reviewer_verdict": _fail("verdict: REQUEST_CHANGES")}, + ) + self._patch_worktree(monkeypatch, tmp_path, "ET-001", "12-review.md", _REVIEW_MD) + task_id = _make_task("review") + advance_stage(task_id, "review", "enduro-trails", "ET-001", + "feature/ET-001-x", finished_agent="reviewer") + contents = _job_contents() + assert len(contents) == 1 + desc = contents[0] + # AC-1: verbatim P0/P1 findings. + assert "Гонка в claim_next_job: отсутствует guard в WHERE" in desc + assert "Нет обработки OSError при чтении отчёта" in desc + # P2 must not leak. + assert "Переименовать blocks" not in desc + # AC-3: link to full file preserved. + assert "docs/work-items/ET-001/12-review.md" in desc + + def test_tc10_tester_embeds_reason_excerpt_and_link(self, monkeypatch, tmp_path): + """TC-10: tester FAIL -> task_desc has reason + report excerpt + link.""" + monkeypatch.setattr( + stage_engine, "QG_CHECKS", + {**stage_engine.QG_CHECKS, + "check_tests_passed": _fail("1 test failed")}, + ) + self._patch_worktree( + monkeypatch, tmp_path, "ET-001", "13-test-report.md", _REPORT_MD + ) + task_id = _make_task("testing") + advance_stage(task_id, "testing", "enduro-trails", "ET-001", + "feature/ET-001-x", finished_agent="tester") + contents = _job_contents() + assert len(contents) == 1 + desc = contents[0] + # AC-2: gate reason present. + assert "1 test failed" in desc + # AC-2: report body excerpt (pytest output) present. + assert "FAILED tests/test_stage_engine.py::TestTaskDescEmbedding" in desc + # AC-3: link to full file preserved. + assert "docs/work-items/ET-001/13-test-report.md" in desc + + def test_tc11_reviewer_graceful_fallback_when_no_file(self, monkeypatch, tmp_path): + """TC-11: missing/broken 12-review.md -> graceful link-only fallback (AC-4).""" + monkeypatch.setattr( + stage_engine, "QG_CHECKS", + {**stage_engine.QG_CHECKS, + "check_reviewer_verdict": _fail("verdict: REQUEST_CHANGES")}, + ) + # Worktree resolves but the review file does not exist. + monkeypatch.setattr( + stage_engine, "get_worktree_path", lambda repo, branch: str(tmp_path) + ) + task_id = _make_task("review") + res = advance_stage(task_id, "review", "enduro-trails", "ET-001", + "feature/ET-001-x", finished_agent="reviewer") + # Rollback still happens exactly as before. + assert res.rolled_back_to == "development" + assert _stage(task_id) == "development" + contents = _job_contents() + assert len(contents) == 1 + desc = contents[0] + # Falls back to the previous link-string behavior (no findings block). + assert "Fix findings in docs/work-items/ET-001/12-review.md" in desc + assert "Findings (P0/P1):" not in desc + + def test_tc11_tester_graceful_fallback_keeps_reason(self, monkeypatch, tmp_path): + """AC-2/AC-4: missing report -> reason still present, link fallback.""" + monkeypatch.setattr( + stage_engine, "QG_CHECKS", + {**stage_engine.QG_CHECKS, + "check_tests_passed": _fail("2 tests failed")}, + ) + monkeypatch.setattr( + stage_engine, "get_worktree_path", lambda repo, branch: str(tmp_path) + ) + task_id = _make_task("testing") + advance_stage(task_id, "testing", "enduro-trails", "ET-001", + "feature/ET-001-x", finished_agent="tester") + desc = _job_contents()[0] + assert "2 tests failed" in desc + assert "docs/work-items/ET-001/13-test-report.md" in desc + + def test_tc12_retry_and_rollback_behavior_unchanged(self, monkeypatch, tmp_path): + """TC-12 (AC-6): embedding does not change retry/rollback semantics. + + 4th developer attempt still alerts instead of enqueueing, even with a + valid review file present. + """ + monkeypatch.setattr( + stage_engine, "QG_CHECKS", + {**stage_engine.QG_CHECKS, + "check_reviewer_verdict": _fail("verdict: REQUEST_CHANGES")}, + ) + self._patch_worktree(monkeypatch, tmp_path, "ET-001", "12-review.md", _REVIEW_MD) + task_id = _make_task("review") + _add_developer_runs(task_id, 3) # already at the cap + res = advance_stage(task_id, "review", "enduro-trails", "ET-001", + "feature/ET-001-x", finished_agent="reviewer") + assert res.rolled_back_to == "development" + assert res.alerted is True + assert stage_engine.send_telegram.called + # No new developer job past the cap, regardless of embedding. + assert _jobs() == [] + + # --------------------------------------------------------------------------- # BUG 8: deploy verdict gates deploy -> done (not the LLM exit code) # ---------------------------------------------------------------------------