fix(gateway): neutral orchestration wording for block-loop triage pings
A repeated-block circuit breaker routes a task to triage and establishes that orchestration attention is needed — it does not establish that a human decision exists. The notifier unconditionally rendered every block_loop_detected event as "needs a human decision", overclaiming owner intent for dependency waits, capability gaps, and transient failures. Branch on the typed block kind in the event payload: only needs_input (the one kind carrying a concrete question for the owner) keeps the decision wording; dependency/capability/transient/None get neutral orchestration wording. TRIAGE visibility, the reason, and recurrence count are preserved. Closes #111125
This commit is contained in:
@@ -367,6 +367,26 @@ def _fmt_changes_requested(ev, n) -> tuple:
|
||||
return msg, None, reason_text
|
||||
|
||||
|
||||
def _fmt_block_loop_detected(ev, n) -> tuple:
|
||||
"""Re-blocked for the same cause past the limit and routed to `triage`.
|
||||
|
||||
It emits no blocked/status event, so ping loudly here. A repeated-block
|
||||
circuit breaker establishes that orchestration attention is needed; it
|
||||
does NOT establish that a human decision or owner input exists. Use
|
||||
neutral orchestration wording unless the block was typed as a genuine
|
||||
owner-input request (`needs_input`, the only kind that carries a concrete
|
||||
question for the owner).
|
||||
"""
|
||||
kind = _payload(ev, "kind")
|
||||
decision = kind == "needs_input"
|
||||
msg = (
|
||||
f"🛑 {n.head} routed to TRIAGE — "
|
||||
f"{'needs a human decision' if decision else 'for orchestration attention'}"
|
||||
f"{_clip(ev, 'recurrences', ' (blocked {}x for the same cause)', 200)}{_clip(ev, 'reason', ': {}', 160)}"
|
||||
)
|
||||
return msg, None, None
|
||||
|
||||
|
||||
# archived / unblocked are claimed (so the cursor advances past them) but
|
||||
# intentionally silent (no formatter), and excluded from _WAKE_KINDS so they
|
||||
# never wake the creator.
|
||||
@@ -383,13 +403,7 @@ _EVENT_FORMATTERS: dict[str, Callable[[Any, "_KanbanNotification"], tuple]] = {
|
||||
"status": lambda ev, n: (f"🔄 {n.head} → {_payload(ev, 'status') or ''}", None, None),
|
||||
"review_requested": _fmt_review_requested,
|
||||
"changes_requested": _fmt_changes_requested,
|
||||
# Re-blocked for the same cause past the limit and routed to `triage` for a
|
||||
# human. It emits no blocked/status event, so ping loudly here.
|
||||
"block_loop_detected": lambda ev, n: (
|
||||
f"🛑 {n.head} routed to TRIAGE — needs a human decision"
|
||||
f"{_clip(ev, 'recurrences', ' (blocked {}x for the same cause)', 200)}{_clip(ev, 'reason', ': {}', 160)}",
|
||||
None, None,
|
||||
),
|
||||
"block_loop_detected": _fmt_block_loop_detected,
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -2,6 +2,8 @@ import asyncio
|
||||
import sqlite3
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
from gateway.config import Platform
|
||||
from gateway.kanban_watchers_common import (
|
||||
@@ -627,6 +629,66 @@ def test_notifier_delivers_block_loop_detected_triage_ping(tmp_path, monkeypatch
|
||||
assert remaining == []
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# #111125 — a repeated-block circuit breaker establishes that orchestration
|
||||
# attention is needed, NOT that a human decision exists. The formatter must
|
||||
# use neutral wording unless the block was typed as a genuine owner-input
|
||||
# request (`needs_input`, the only kind that carries a concrete question).
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class _StubEvent:
|
||||
def __init__(self, payload):
|
||||
self.payload = payload
|
||||
|
||||
|
||||
class _StubNotif:
|
||||
head = "H123"
|
||||
|
||||
|
||||
def _fmt_block_loop(payload):
|
||||
from gateway.kanban_watchers_notifier import _EVENT_FORMATTERS
|
||||
|
||||
msg, _, _ = _EVENT_FORMATTERS["block_loop_detected"](_StubEvent(payload), _StubNotif())
|
||||
return msg
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"kind",
|
||||
["dependency", "capability", "transient", None],
|
||||
)
|
||||
def test_block_loop_non_owner_input_uses_neutral_orchestration_wording(kind):
|
||||
"""Orchestration/technical block kinds must NOT claim a human decision.
|
||||
|
||||
Regression for #111125: before this fix every `block_loop_detected` was
|
||||
rendered as \"needs a human decision\" regardless of its typed reason. A
|
||||
dependency wait, capability gap, or transient failure routed to triage is
|
||||
an orchestration handoff with no question for the owner.
|
||||
"""
|
||||
payload = {"reason": "waiting on upstream", "kind": kind, "recurrences": 2}
|
||||
msg = _fmt_block_loop(payload)
|
||||
assert "for orchestration attention" in msg
|
||||
assert "needs a human decision" not in msg
|
||||
# Circuit-breaker visibility is preserved.
|
||||
assert "TRIAGE" in msg
|
||||
assert "waiting on upstream" in msg
|
||||
|
||||
|
||||
def test_block_loop_owner_input_keeps_decision_wording():
|
||||
"""A `needs_input` block carries a concrete question for the owner, so the
|
||||
owner-decision wording is correct and must be retained (#111125)."""
|
||||
payload = {
|
||||
"reason": "Which API key should this use?",
|
||||
"kind": "needs_input",
|
||||
"recurrences": 2,
|
||||
"limit": kb.BLOCK_RECURRENCE_LIMIT,
|
||||
}
|
||||
msg = _fmt_block_loop(payload)
|
||||
assert "needs a human decision" in msg
|
||||
assert "for orchestration attention" not in msg
|
||||
assert "Which API key should this use?" in msg
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Handoffs that hand a decision back to the origin must wake it, not only ping
|
||||
# it: `review_requested` (implementation done, waiting for a reviewer) and
|
||||
|
||||
Reference in New Issue
Block a user