fix(reconciler): terminal-skip + state_uuid dedup on F-1 path
Закрывает 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 <noreply@anthropic.com>
This commit is contained in:
@@ -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}
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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}"
|
||||
|
||||
Reference in New Issue
Block a user