From e729b7117f90938b7668a1098da74e99bdf6af08 Mon Sep 17 00:00:00 2001 From: claude-bot Date: Tue, 9 Jun 2026 02:17:59 +0300 Subject: [PATCH] fix(reconciler): terminal-skip + state_uuid dedup on F-1 path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Закрывает F-1-пробел ORCH-068: терминал-исключение и in-memory dedup (изначально только F-2) распространены на gate-side путь реконсилятора, устраняя ложное «🔧 reconciler: ET-002 done разблокирована (потерян webhook)» (особенно после рестарта). - D1: новый _resolve_issue_status — один сетевой резолв Plane-статуса задачи за тик (states, groups, state_uuid) после дешёвых локальных гардов; never-raise -> ({}, {}, None) при сбое. - D2: безусловный терминал-скип ДО Guard 2 (группа Plane completed/ cancelled, fallback на логические ключи done/cancelled, либо стадия в БД орка ∈ {done, cancelled}); skipped_terminal_total++, не подчинён reconcile_skip_blocked_enabled. - D3: _is_blocked_or_needs_input переиспользует резолв D1 (опц. аргументы, _UNSET -> самостоятельный резолв для прямых/легаси-вызовов; 1:1). - D4: вызов _note_unblock на F-1 теперь передаёт state_uuid -> dedup работает на обоих путях (deduped_total++ на повторе). Анти-регресс: легитимный unblock не-терминальной застрявшей задачи по-прежнему advance + один Telegram. STAGE_TRANSITIONS / QG_CHECKS / схема БД / сигнатуры advance_*/_note_unblock / форма status() / новые флаги — без изменений; never-raise сохранён. Тесты: tests/test_reconciler.py TC-86-01..09/11, tests/test_reconciler_plane.py TC-86-10. Полный прогон зелёный (1069). Refs: ORCH-086 Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 1 + src/reconciler.py | 97 ++++++++++++-- tests/test_orch026_task_deps.py | 8 +- tests/test_reconciler.py | 230 ++++++++++++++++++++++++++++++++ tests/test_reconciler_plane.py | 15 +++ 5 files changed, 336 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc58e7c..cd27836 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ Формат: [Keep a Changelog](https://keepachangelog.com/). Записи — на смысловой PR/задачу. ## [Unreleased] +- **Терминал-скип и `state_uuid`-dedup на пути F-1 реконсилятора** (ORCH-086, `fix`): в Telegram периодически (особенно после рестарта орка) прилетало ложное `🔧 reconciler: ET-002 done разблокирована (потерян webhook)` — задача давно завершена, ничего не разблокируется, это шум. **Корень:** ORCH-068 закрыл livelock только на F-2 (plane-side); путь F-1 (gate-side) остался непокрытым по двум причинам — (A) вызов `_note_unblock(work_item_id, stage)` шёл без `state_uuid`, поэтому in-memory dedup пропускался; (B) единственным «терминал-фильтром» F-1 была выборка `get_active_tasks_for_reconcile` (`WHERE stage != 'done'`), не знающая о статусе issue в Plane — задача с дрейфом «БД орка не-`done`, а Plane уже `Done`» проходила фильтр, no-op условные гейты (enduro) давали зелёный → `advance` → ложное уведомление. **Фикс (ADR-001, локализован в `src/reconciler.py`):** (D1) новый `_resolve_issue_status(task)` делает **один** сетевой резолв Plane-статуса задачи за тик `(states, groups, state_uuid)` после дешёвых локальных гардов (busy/young/escalated в Plane не ходят), never-raise → `({}, {}, None)` при сбое; (D2) безусловный терминал-скип ДО Guard 2 — терминальная задача (группа Plane `completed`/`cancelled`, fallback на логические ключи `done`/`cancelled`, ЛИБО стадия в БД орка ∈ `{done, cancelled}`, т.к. `cancelled` не отсекается выборкой) → ранний `return` + `skipped_terminal_total++`, не подчинён `reconcile_skip_blocked_enabled` (тот гейтит только Guard 2); (D3) `_is_blocked_or_needs_input` переиспользует резолв D1 (3-й/4-й опц. аргументы; при `_UNSET` — самостоятельный резолв для прямых/легаси-вызовов, поведение 1:1); (D4) вызов `_note_unblock` на F-1 теперь передаёт `state_uuid` → dedup работает и на F-1 (повтор того же `issue_id`+`state_uuid` → `deduped_total++`, без второго Telegram). Терминальность — тот же `_is_terminal_state`, что и в F-2 (первичный дискриминатор — группа Plane, устойчив к UUID-алиасингу/мультипроектности; покрывает enduro и orchestrator). Анти-регресс (AC-4): легитимный unblock реально застрявшей не-терминальной задачи по-прежнему `advance` + ровно один Telegram (`unblocked_total++`). `STAGE_TRANSITIONS`, `QG_CHECKS`, схема БД, сигнатуры `advance_stage`/`advance_if_gate_passed`/`_note_unblock`, форма `status()`/`GET /queue`, новые config-флаги — без изменений; never-raise сохранён. ADR `docs/work-items/ORCH-086/06-adr/ADR-001-reconciler-f1-terminal-skip-and-dedup.md`. Тесты: `tests/test_reconciler.py` (TC-86-01..09/11: терминал по группе completed/cancelled, fallback по логическому ключу, DB-side cancelled, проброс/dedup `state_uuid`, анти-регресс, never-raise, независимость от Guard-2-флага), `tests/test_reconciler_plane.py` (TC-86-10: форма `status()` неизменна). Документация: `docs/architecture/README.md` (раздел Reconciler F-1). - **Подавление Telegram link-preview в карточке трекера / уведомлениях** (ORCH-080): под каждой карточкой трекера (`bump` и `edit`) и под notify/alert-сообщениями Telegram разворачивал баннер «Plane — Modern project management» для кликабельной ссылки `ORCH-NNN` на issue. В дефолтном `bump`-режиме (ORCH-067) карточка пересоздаётся на каждом переходе → баннер дублировался и засорял ленту (жалоба Owner, 08.06). **Корень:** JSON-payload обоих низкоуровневых примитивов `notifications.send_telegram` (`POST /sendMessage`) и `notifications.edit_telegram` (`POST /editMessageText`) не содержал ключ `disable_web_page_preview`. **Фикс (ADR-001, минимальная аддитивная правка на уровне примитива):** добавлен `"disable_web_page_preview": True` в payload обоих методов — гасит баннер у ВСЕХ потребителей (`update_task_tracker` в обоих режимах, `notify_approve_requested`, `notify_error`, alert'ы стадий из `launcher`/`stage_engine`) без изменения их кода. Безусловно, без kill-switch (превью трекера не нужно никому, риск нулевой). `parse_mode: "HTML"` сохранён в обоих payload → ссылка `ORCH-NNN` остаётся кликабельной; `disable_notification` (карточка тихая), bump/edit-логика, инвариант «одна карточка на задачу», контракты возврата (`send_telegram → message_id|None`, `edit_telegram → EDIT_*`) и never-raise — не затронуты. `STAGE_TRANSITIONS`, `QG_CHECKS`, схема БД — без изменений. ADR `docs/work-items/ORCH-080/06-adr/ADR-001-disable-telegram-link-preview.md`. Тесты: `tests/test_link_preview_disabled.py` (TC-01..06: флаг в обоих payload, регрессия `parse_mode`/полей, контракты возврата, never-raise). Документация: `CLAUDE.md` + `docs/architecture/README.md` (компонент Notifications). - **Гарантированный идемпотентный код-PR перед merge-verify (фикс ложного HOLD «no open PR»)** (ORCH-082/ORCH-81): закрыт отсутствующий инвариант «к моменту merge-verify у ветки есть открытый код-PR». **Корень (ORCH-074, 08.06):** PR создавался единственной `launcher._ensure_pr` ТОЛЬКО на developer-пути и ТОЛЬКО при свежем worktree-коммите (`exit==0 → git status непуст → commit → push → agent=="developer"`); после ручных восстановлений `main` у ветки ORCH-074 не оказалось открытого код-PR → детерминированный `merge_gate.merge_pr` вернул `("False", "no open PR")` → защита ORCH-073 верно удержала задачу (HOLD, не ложный `done`), но лечила следствие. **Фикс (ADR-001, аддитивно, внутри того же под-гейта merge-verify, машина стадий не тронута):** (1) новый идемпотентный leaf-актор `merge_gate.ensure_open_pr(repo, branch) -> (status, detail)` (never-raise): `GET …/pulls?state=open` с фильтром **`head.ref==branch` И `base.ref=="main"`** (идентичен `merge_pr`/ORCH-073 FR-3 — авто-docs-PR `base!=main` НЕ код-PR) → `("existed", N)`; иначе `POST …/pulls` → `("created", N)`; гонка `409/422` «PR exists» → повторный GET → `existed` (без дублей); любая иная HTTP/parse/сетевая ошибка → `("failed", reason)`. (2) Врезка в `stage_engine._handle_merge_verify` ПОСЛЕ резолва `validated_revision` и ПЕРЕД `merge_pr`: при `merge_verify_autocreate_pr_enabled` → `ensure_open_pr`; `created|existed` → штатно к `merge_pr` → `verify_merged_to_main`; `failed` → честный HOLD через новый helper `_hold_pr_create_failed` (текст «PR создать не удалось», `result.note="pr-create-failed-hold"` — текстуально отличим от not-merged HOLD; задача остаётся на `deploy`, НЕ `done`, БЕЗ отката на development). (3) `launcher._ensure_pr` делегирован в `merge_gate.ensure_open_pr` (единый код создания PR, общий фильтр `head==branch & base==main`); триггер «создавать только на developer-пути со свежим коммитом» НЕ ужесточён — менялась только реализация под капотом. **Защита ORCH-073 неприкосновенна и приоритетна:** подтверждение merge остаётся ТОЛЬКО `verify_merged_to_main` (SHA-в-main) + `check_main_regression`; `ensure_open_pr` устраняет лишь ЛОЖНЫЙ HOLD «no open PR», реально невлитый код → HOLD как прежде. Kill-switch `ORCH_MERGE_VERIFY_AUTOCREATE_PR_ENABLED` (дефолт `true`); область — `merge_verify_applies(repo)` (self-hosting / `merge_verify_repos`), non-self → no-op; `false` → поведение ORCH-074 1:1. Идемпотентность из Gitea (наличие открытого PR), без миграции БД (restart-safe); `main` не push/force-push. Инварианты НЕ менялись: `STAGE_TRANSITIONS`, реестр `QG_CHECKS` (под-гейт — врезка в `advance_stage`, не новый QG), схема БД, `check_deploy_status`/`_parse_deploy_status`, exit-коды хука, merge-gate (ORCH-043), image-freshness (ORCH-058), внешние HTTP-эндпоинты. ADR `docs/work-items/ORCH-082/06-adr/ADR-001-ensure-open-pr-before-merge-verify.md` (+ сквозной `adr-0016`). Документация: `docs/architecture/README.md` (блок ORCH-082 в merge-verify). Тесты: `tests/test_orch082_ensure_pr.py` (TC-01..05: идемпотентный актор, фильтр base==main, гонка 409/422, never-raise), `tests/test_orch082_merge_verify_autocreate.py` (TC-06..12: врезка, регресс ORCH-073, kill-switch, условность, наблюдаемость). - **Устойчивость резолва `--effort` к пустому env + developer → `xhigh`** (ORCH-081/ORCH-52h): фикс конфигурационного бага, из-за которого в проде `resolve_agent_effort()` возвращал `''` для всех 6 агентов и `--effort` не передавался в Claude CLI (каждый агент бежал на встроенном CLI-дефолте вместо заявленного уровня — прямой удар по предсказуемости качества всего конвейера, включая enduro-trails из общего инстанса). **Корень:** pydantic Settings трактует ПРИСУТСТВУЮЩУЮ env-переменную, даже пустую (`ORCH_AGENT_EFFORT_*=` без значения), как явное `''` и перебивает class-default; в проде пусты И per-agent, И `agent_effort_default`, поэтому у цепочки резолва (`_resolve_agent_attr`: project-override → per-agent env → default → `''`) не остаётся непустого «пола» для отката. **Фикс (вариант c, ADR-001):** в `resolve_agent_effort` (`src/agents/launcher.py`) добавлен уровень 4 — непустой **per-role floor** ниже `default`: новый чистый helper `_agent_effort_floor(agent)` возвращает декларированный class-default поля `agent_effort_` через `type(settings).model_fields[...].default` (значение, которое пустой env перебить НЕ может). Floor срабатывает ТОЛЬКО когда уровни 1–3 пусты и применяется ДО валидации, поэтому: (а) при пустом прод-`.env` каждая роль получает СВОЙ канонический уровень (developer=`xhigh`, tester/deployer=`medium`, analyst/architect/reviewer=`high`), а не общий default; (б) явная опечатка (`turbo`/`ultra`) непуста → floor НЕ применяется → значение штатно дропается валидацией `VALID_EFFORTS` в `''` (never-break ORCH-41 не регрессирует, floor не маскирует мусор); (в) непустой явный env/project-override/`default` по-прежнему ПОБЕЖДАЕТ floor (приоритет резолва сохранён 1:1). Unknown-agent (имя вне 6 ролей) деградирует на class-default `agent_effort_default` (`high`) — безопасный непустой пол. **`config.py`:** `agent_effort_developer` `high → xhigh` (канон Opus 4.8: coding/agentic роль) — единственное изменение значений; floor подтягивает его автоматически (единый источник правды, ноль риска дрейфа floor-карты). Инварианты НЕ менялись: приоритеты/сигнатуры резолва ORCH-41, `_resolve_agent_attr` (общий с model-резолвом, не тронут), `resolve_agent_model` (ORCH-074), путь проброса `--effort` в `_spawn`, `VALID_EFFORTS`, API, схема БД (без миграций). ADR `docs/work-items/ORCH-081/06-adr/ADR-001-effort-resolution-floor.md`. Документация: `docs/architecture/README.md` (таблица «модель/эффорт по ролям»: developer `xhigh` + ремарка про floor), `.env.example` (`ORCH_AGENT_EFFORT_DEVELOPER=xhigh` + комментарий split/floor). Тесты: `tests/test_resolve_agent_effort.py` (TC-01..08: канон-дефолты, floor при пустом env per-role, floor-не-маскирует-typo, приоритет, `xhigh∈VALID_EFFORTS`, сборка флага `--effort xhigh`/`--effort medium`). diff --git a/src/reconciler.py b/src/reconciler.py index a442f54..315cb6d 100644 --- a/src/reconciler.py +++ b/src/reconciler.py @@ -73,6 +73,11 @@ from . import task_deps logger = logging.getLogger("orchestrator.reconciler") +# ORCH-086 (D3): sentinel distinguishing "caller did not pass a pre-resolved +# state_uuid" (Guard 2 self-resolves, backward-compatible 1-arg call) from an +# explicit ``None`` (Plane unreachable -> conservative skip). +_UNSET = object() + def _parse_grace_overrides(raw: str) -> dict[str, int]: """Parse ``reconcile_grace_overrides_json`` into {stage: seconds}. @@ -183,6 +188,14 @@ class Reconciler: # AC-16: analysis is a human gate -> owned by F-2, never F-1. if stage == "analysis": return + # ORCH-086 D2 (DB-side terminal drift): ``get_active_tasks_for_reconcile`` + # filters ``stage != 'done'`` but NOT ``cancelled``. A task already + # terminal in the orchestrator DB is fully in sync by definition -> skip + # before any gate/network work, mirroring the F-2 terminal-skip counter + # (single semantics with ``_reconcile_plane_issue``). Local, no network. + if stage in ("done", "cancelled"): + self.skipped_terminal_total += 1 + return # created / done have no gate to evaluate. if get_qg_for_stage(stage) is None: return @@ -201,9 +214,25 @@ class Reconciler: # Deterministic, local SQL, no network — and checked FIRST (cheapest). if developer_retry_count(task_id) >= MAX_DEVELOPER_RETRIES: return + # ORCH-086 D1: single networked resolve per task per tick, AFTER the cheap + # local guards (so busy/young/escalated tasks never hit Plane). Feeds the + # Plane-side terminal-skip (D2), Guard 2 (D3) and the state_uuid handed to + # _note_unblock (D4) — no duplicate fetch. + states, groups, state_uuid = self._resolve_issue_status(task) + # ORCH-086 D2 (Plane-side terminal-skip), UNCONDITIONAL (not gated by + # reconcile_skip_blocked_enabled, which gates ONLY Guard 2). A task whose + # Plane status is terminal (group completed/cancelled, or the logical + # done/cancelled fallback) is fully in sync -> never a real unblock. + # Runs BEFORE Guard 2 so terminal tasks correctly bump skipped_terminal_total + # instead of being swallowed by Guard 2's conservative path. Closes the F-1 + # gap of ORCH-068 (which only covered F-2); fixes the spurious + # "ET-002 ... разблокирована" notification. + if self._is_terminal_state(state_uuid, states, groups): + self.skipped_terminal_total += 1 + return # ORCH-060 Guard 2: respect an explicit human gate (Blocked / Needs Input). - # Networked; runs after Guard 1 so escalated tasks never hit Plane. - if self._is_blocked_or_needs_input(task): + # Reuses the D1 resolve (ORCH-086 D3) so the tick makes a single fetch. + if self._is_blocked_or_needs_input(task, states, state_uuid): return # ORCH-026 Guard 3 (B-5): a task blocked by an unfinished declared # dependency is legitimately waiting, NOT stuck -> F-1 must not advance it @@ -225,9 +254,48 @@ class Reconciler: task.get("branch") or "", ) if result is not None and getattr(result, "advanced", False): - self._note_unblock(task.get("work_item_id") or str(task_id), stage) + # ORCH-086 D4: pass state_uuid so the in-memory dedup guard covers F-1 + # too (a repeat tick for the same issue+state is suppressed; survives + # the "first pass after restart" symptom together with the D2 skip). + self._note_unblock( + task.get("work_item_id") or str(task_id), stage, state_uuid + ) - def _is_blocked_or_needs_input(self, task: dict) -> bool: + def _resolve_issue_status( + self, task: dict + ) -> tuple[dict, dict, str | None]: + """ORCH-086 D1: one networked resolve per task per tick. + + Returns ``(states, groups, current_state_uuid)``. A single + ``fetch_issue_state`` plus the cached (ORCH-068 TTL) + ``get_project_states`` / ``get_project_state_groups``. The result feeds + the terminal-skip (D2), Guard 2 (D3) and the ``state_uuid`` handed to + ``_note_unblock`` (D4), so the tick never fetches the same issue twice. + + **never-raise.** On any failure / unresolved project / missing state -> + ``({} or states, {} or groups, None)`` so callers apply their + conservative fallback (terminal-skip = not terminal; Guard 2 = skip). + """ + try: + proj = projects.get_project_by_repo(task.get("repo") or "") + if proj is None: + return {}, {}, None + pid = proj.plane_project_id + states = get_project_states(pid) + groups = get_project_state_groups(pid) + issue_id = task.get("plane_id") or task.get("plane_issue_id") or "" + state_uuid = fetch_issue_state(issue_id, pid) + return states or {}, groups or {}, state_uuid + except Exception as e: # noqa: BLE001 - never break the tick + logger.warning( + f"reconciler D1: status resolve failed for task " + f"{task.get('id')}, treating as unresolved: {e}" + ) + return {}, {}, None + + def _is_blocked_or_needs_input( + self, task: dict, states: dict | None = None, state_uuid=_UNSET + ) -> bool: """Guard 2 (ORCH-060 + ORCH-066): is this issue waiting for a human OR in an active orchestrator wait that F-1 must not "revive"? @@ -251,19 +319,22 @@ class Reconciler: human-gated task re-introduces the bounce we are trying to kill. The sub-flag ``reconcile_skip_blocked_enabled`` disables ONLY this networked guard (escape hatch for a Plane outage); Guard 1 stays active. + + **ORCH-086 D3:** the production caller (``_reconcile_gate_task``) passes + the already-resolved ``(states, state_uuid)`` from the single D1 fetch, so + the tick does not hit Plane twice. When ``state_uuid`` is left ``_UNSET`` + (direct/legacy 1-arg call) Guard 2 self-resolves via ``_resolve_issue_status`` + — behaviour identical to the pre-ORCH-086 code. """ if not settings.reconcile_skip_blocked_enabled: return False try: - proj = projects.get_project_by_repo(task.get("repo") or "") - if proj is None: - return True # cannot resolve the project -> conservative skip - pid = proj.plane_project_id - states = get_project_states(pid) - issue_id = task.get("plane_id") or task.get("plane_issue_id") or "" - cur = fetch_issue_state(issue_id, pid) - if cur is None: - return True # Plane unreachable / no state -> conservative skip + if state_uuid is _UNSET: + # Backward-compatible self-resolve (direct callers / tests). + states, _groups, state_uuid = self._resolve_issue_status(task) + if not states or state_uuid is None: + return True # unresolved project / Plane unreachable -> conservative skip + cur = state_uuid # ORCH-066 BR-13: active orchestrator waits, minus base working # statuses so aliased (enduro) keys never widen the skip-set. base_working = { diff --git a/tests/test_orch026_task_deps.py b/tests/test_orch026_task_deps.py index d9d9146..0d681f8 100644 --- a/tests/test_orch026_task_deps.py +++ b/tests/test_orch026_task_deps.py @@ -148,8 +148,12 @@ def test_reconciler_skip_helper_honours_block(monkeypatch): monkeypatch.setattr(rec.settings, "reconcile_grace_default_s", 0, raising=False) r = rec.Reconciler() - # Bypass Guard 2 (networked) so we isolate Guard 3. - monkeypatch.setattr(r, "_is_blocked_or_needs_input", lambda task: False) + # Bypass Guard 2 (networked) so we isolate Guard 3. ORCH-086: the production + # call now passes the resolved (states, state_uuid), so accept extra args. + monkeypatch.setattr(r, "_is_blocked_or_needs_input", lambda *a, **k: False) + # ORCH-086: the D1 resolve now runs before Guard 2 (for the terminal-skip) — + # keep it offline so this Guard-3 test stays deterministic. + monkeypatch.setattr(r, "_resolve_issue_status", lambda task: ({}, {}, None)) task_row = {"id": b, "stage": "development", "repo": "orchestrator", "work_item_id": "ORCH-51", "branch": "feature/ORCH-51", "age_s": 9999} diff --git a/tests/test_reconciler.py b/tests/test_reconciler.py index f28489a..e587737 100644 --- a/tests/test_reconciler.py +++ b/tests/test_reconciler.py @@ -744,3 +744,233 @@ def test_tc21_guard2_aliased_waits_do_not_widen_skipset(monkeypatch): assert _guard2(monkeypatch, aliased, "done-u") is False # The explicit human gates still skip. assert _guard2(monkeypatch, aliased, "blocked-u") is True + + +# =========================================================================== +# ORCH-086: terminal-skip + state_uuid dedup on the F-1 (gate-side) path. +# Closes the gap of ORCH-068 (which covered only F-2). The spurious +# "ET-002 ... разблокирована (потерян webhook)" notification for a task that is +# already terminal in Plane (but drifted in the orchestrator DB) is suppressed. +# =========================================================================== +def _plane_terminal(monkeypatch, *, state_uuid="done-uuid", + states=None, groups=None): + """Make Plane report ``state_uuid`` as the issue's current state, with the + given {key->uuid} states and {uuid->group} groups maps.""" + monkeypatch.setattr(reconciler_mod, "fetch_issue_state", + MagicMock(return_value=state_uuid)) + monkeypatch.setattr(reconciler_mod, "get_project_states", + MagicMock(return_value=states if states is not None + else {"done": "done-uuid"})) + monkeypatch.setattr(reconciler_mod, "get_project_state_groups", + MagicMock(return_value=groups if groups is not None + else {"done-uuid": "completed"})) + + +# --- TC-86-01 (AC-1) ------------------------------------------------------- +def test_tc86_01_terminal_in_plane_not_unblocked(monkeypatch): + """enduro task NOT-done in the DB but terminal in Plane (group=completed), + green gate: F-1 must NOT call _note_unblock / send_telegram — neither on a + normal tick nor on the first pass of a fresh Reconciler (clean dedup).""" + _green_ci(monkeypatch) + monkeypatch.setattr(reconciler_mod.settings, "reconcile_notify_unblock", True) + tg = MagicMock() + monkeypatch.setattr(reconciler_mod, "send_telegram", tg) + note = MagicMock() + monkeypatch.setattr(reconciler_mod.Reconciler, "_note_unblock", note) + _plane_terminal(monkeypatch) # Plane says Done (group=completed) + + task_id = _make_task("development", wi="ET-002", age_s=3600) + + # Fresh Reconciler -> empty _unblock_dedup -> the "first pass after restart" + # symptom is exercised; the terminal-skip must fire regardless of dedup. + rec = Reconciler() + rec.reconcile_gate_once() + rec.reconcile_gate_once() + + assert _stage_of(task_id) == "development" # never advanced + note.assert_not_called() + tg.assert_not_called() + assert rec.unblocked_total == 0 + assert rec.skipped_terminal_total >= 1 + + +# --- TC-86-02 (AC-2) ------------------------------------------------------- +def test_tc86_02_terminal_skip_counter_no_advance(monkeypatch): + """Terminal-skip bumps skipped_terminal_total and never reaches + advance_if_gate_passed.""" + spy = MagicMock() + monkeypatch.setattr(reconciler_mod, "advance_if_gate_passed", spy) + _plane_terminal(monkeypatch) + + _make_task("development", wi="ET-002", age_s=3600) + rec = Reconciler() + rec.reconcile_gate_once() + + assert rec.skipped_terminal_total == 1 + spy.assert_not_called() + + +# --- TC-86-03 (AC-2 / R1) -------------------------------------------------- +def test_tc86_03_terminal_by_group_cancelled(monkeypatch): + """Terminal detection by Plane state GROUP works for cancelled too, and is + project-independent (group discriminator, not a per-project key).""" + spy = MagicMock() + monkeypatch.setattr(reconciler_mod, "advance_if_gate_passed", spy) + _plane_terminal( + monkeypatch, state_uuid="cancel-uuid", + states={"done": "done-uuid", "cancelled": "cancel-uuid"}, + groups={"cancel-uuid": "cancelled"}, + ) + + _make_task("development", wi="ET-002", age_s=3600) + rec = Reconciler() + rec.reconcile_gate_once() + + assert rec.skipped_terminal_total == 1 + spy.assert_not_called() + + +# --- TC-86-04 (AC-2 / R1) -------------------------------------------------- +def test_tc86_04_terminal_fallback_logical_key_empty_groups(monkeypatch): + """Fallback when groups are unavailable ({}): terminality by the project's + logical done/cancelled key.""" + spy = MagicMock() + monkeypatch.setattr(reconciler_mod, "advance_if_gate_passed", spy) + _plane_terminal( + monkeypatch, state_uuid="done-key-uuid", + states={"done": "done-key-uuid", "cancelled": "cancel-key-uuid"}, + groups={}, # group unknown -> logical-key fallback + ) + + _make_task("development", wi="ET-002", age_s=3600) + rec = Reconciler() + rec.reconcile_gate_once() + + assert rec.skipped_terminal_total == 1 + spy.assert_not_called() + + +# --- TC-86-05 (AC-2) ------------------------------------------------------- +def test_tc86_05_terminal_by_db_stage_cancelled(monkeypatch): + """DB-side terminal drift: a task with stage='cancelled' (NOT filtered by + get_active_tasks_for_reconcile, which only drops 'done') is skipped locally + without reaching _note_unblock / advance — and bumps skipped_terminal_total.""" + spy = MagicMock() + monkeypatch.setattr(reconciler_mod, "advance_if_gate_passed", spy) + note = MagicMock() + monkeypatch.setattr(reconciler_mod.Reconciler, "_note_unblock", note) + # A networked resolve must not even be needed for the DB-side guard. + monkeypatch.setattr( + reconciler_mod, "fetch_issue_state", + MagicMock(side_effect=AssertionError("must not hit Plane for DB-cancelled")), + ) + + _make_task("cancelled", wi="ET-002", age_s=3600) + rec = Reconciler() + rec.reconcile_gate_once() + + assert rec.skipped_terminal_total == 1 + spy.assert_not_called() + note.assert_not_called() + + +# --- TC-86-06 (AC-3) ------------------------------------------------------- +def test_tc86_06_legit_unblock_passes_state_uuid(monkeypatch): + """A legitimate unblock calls _note_unblock with a non-empty state_uuid; the + dedup guard stores issue_id -> state_uuid.""" + _green_ci(monkeypatch) + # Default fixture: fetch_issue_state -> 'some-non-gated-state', groups {} -> + # not terminal, not blocked -> the task advances. + task_id = _make_task("development", wi="ET-300", age_s=3600) + + rec = Reconciler() + rec.reconcile_gate_once() + + assert _stage_of(task_id) == "review" + assert rec.unblocked_total == 1 + assert rec._unblock_dedup.get("ET-300") == "some-non-gated-state" + + +# --- TC-86-07 (AC-3) ------------------------------------------------------- +def test_tc86_07_repeat_tick_deduped(monkeypatch): + """A repeat F-1 tick for the same issue+state_uuid is suppressed by the dedup + guard: deduped_total += 1 and no second send_telegram.""" + monkeypatch.setattr(reconciler_mod.settings, "reconcile_notify_unblock", True) + tg = MagicMock() + monkeypatch.setattr(reconciler_mod, "send_telegram", tg) + # advance "succeeds" but leaves the stage put, so each tick reaches + # _note_unblock again with the SAME resolved state_uuid. + monkeypatch.setattr( + reconciler_mod, "advance_if_gate_passed", + MagicMock(return_value=MagicMock(advanced=True)), + ) + + _make_task("development", wi="ET-301", age_s=3600) + rec = Reconciler() + rec.reconcile_gate_once() # first: notifies + rec.reconcile_gate_once() # second: same issue+state -> deduped + + assert tg.call_count == 1 + assert rec.unblocked_total == 1 + assert rec.deduped_total == 1 + + +# --- TC-86-08 (AC-4, anti-regress) ----------------------------------------- +def test_tc86_08_legit_unblock_still_notifies(monkeypatch): + """A NON-terminal genuinely stuck task (working Plane status, past grace, no + active job, green gate) is STILL advanced and notifies exactly once.""" + _green_ci(monkeypatch) + monkeypatch.setattr(reconciler_mod.settings, "reconcile_notify_unblock", True) + tg = MagicMock() + monkeypatch.setattr(reconciler_mod, "send_telegram", tg) + + task_id = _make_task("development", wi="ET-302", age_s=3600) + rec = Reconciler() + rec.reconcile_gate_once() + + assert _stage_of(task_id) == "review" + tg.assert_called_once() + assert rec.unblocked_total == 1 + assert rec.skipped_terminal_total == 0 + + +# --- TC-86-09 (AC-5, never-raise) ------------------------------------------ +def test_tc86_09_never_raise_no_false_notify(monkeypatch): + """An exception in the terminal-detect / fetch_issue_state path does not blow + up the tick AND does not produce a false unblock (conservative).""" + _green_ci(monkeypatch) + monkeypatch.setattr(reconciler_mod.settings, "reconcile_notify_unblock", True) + tg = MagicMock() + monkeypatch.setattr(reconciler_mod, "send_telegram", tg) + monkeypatch.setattr( + reconciler_mod, "fetch_issue_state", + MagicMock(side_effect=RuntimeError("plane boom")), + ) + + task_id = _make_task("development", wi="ET-303", age_s=3600) + rec = Reconciler() + rec.reconcile_gate_once() # must not raise + + # resolve failed -> state_uuid None -> not terminal, Guard 2 conservative skip. + assert _stage_of(task_id) == "development" + tg.assert_not_called() + assert rec.unblocked_total == 0 + + +# --- TC-86-11 (AC-6) ------------------------------------------------------- +def test_tc86_11_terminal_skip_independent_of_guard2_flag(monkeypatch): + """reconcile_skip_blocked_enabled=False (Guard 2 escape hatch) does NOT + disable the unconditional terminal-skip: a terminal task is still skipped.""" + monkeypatch.setattr( + reconciler_mod.settings, "reconcile_skip_blocked_enabled", False + ) + spy = MagicMock() + monkeypatch.setattr(reconciler_mod, "advance_if_gate_passed", spy) + _plane_terminal(monkeypatch) # group=completed + + _make_task("development", wi="ET-304", age_s=3600) + rec = Reconciler() + rec.reconcile_gate_once() + + assert rec.skipped_terminal_total == 1 + spy.assert_not_called() diff --git a/tests/test_reconciler_plane.py b/tests/test_reconciler_plane.py index 51c96c7..520d9e9 100644 --- a/tests/test_reconciler_plane.py +++ b/tests/test_reconciler_plane.py @@ -684,3 +684,18 @@ def test_tc10_done_silent_on_all_projects(monkeypatch): assert recon.unblocked_total == 0 assert recon.skipped_terminal_total >= 2 # one per project assert _job_count() == 0 + + +# --------------------------------------------------------------------------- +# TC-86-10 (AC-6): the status()/GET-queue observability shape is unchanged by +# ORCH-086 — the ORCH-068 counters (skipped_terminal_total / deduped_total / +# unblocked_total) are still present, so the F-2 regression contract holds. +# --------------------------------------------------------------------------- +def test_tc86_10_status_shape_unchanged(): + snap = Reconciler().status() + for key in ( + "enabled", "plane_enabled", "interval", "last_run_ts", + "unblocked_total", "last_unblocked", + "skipped_terminal_total", "deduped_total", + ): + assert key in snap, f"status() missing observability key: {key}"