fix(orchestrator): break the notification-driven respawn loop (#643)

The escalation/approval dispatchers spawn a notification's recipient every
cooldown window for as long as it stays pending. These spawns carry no
task_id, so the PM respawn breaker never sees them — a single wedged
alert/escalation whose recipient never resolves it respawns that recipient
forever. Observed live: fe-pm's unacked alerts kept main-pm/fe-pm spawning
every ~2-3 min for 6+ hours.

Two guards, both gating the spawn after the existing cooldown:
- A hard per-(agent, notification) attempt cap (notification_spawn_max_attempts,
  default 5): once a notification has respawned its target that many times
  without being acknowledged, stop and log once. The count is id-scoped and
  survives map pruning (re-stamp), so a fresh escalation is unaffected.
- A live-work check before spawning: skip when the notification has expired,
  is stale past notification_spawn_max_age_seconds (default 6h — wedged or
  reloaded from before a restart), or its related task is already terminal.
  Fail-open — a failed task fetch or unparseable field never suppresses a
  real escalation.

Co-authored-by: Renn F <rennf93@users.noreply.github.com>

roboco/config.py

@@ -267,6 +267,29 @@ class Settings(BaseSettings):
             "unacknowledged. 0 disables the damper (legacy every-tick respawn)."
         ),
     )
+    notification_spawn_max_attempts: int = Field(
+        default=5,
+        ge=0,
+        description=(
+            "Hard cap on notification-triggered spawns per (agent, notification): "
+            "past this many attempts without the notification being acknowledged, "
+            "stop re-spawning (the notification-driven analogue of the PM respawn "
+            "breaker — these dispatchers carry no task_id so that breaker never "
+            "sees them). Prevents one wedged escalation/alert from respawning its "
+            "recipient every cooldown window indefinitely. 0 disables the cap."
+        ),
+    )
+    notification_spawn_max_age_seconds: int = Field(
+        default=21600,
+        ge=0,
+        description=(
+            "Skip notification-triggered spawns for a notification older than "
+            "this (default 6h). A still-pending notification this stale is wedged "
+            "or reloaded from before a restart — reviving an agent for it acts on "
+            "dead work. Independent of the per-notification expiry and the "
+            "terminal-related-task check. 0 disables the staleness gate."
+        ),
+    )
     notification_ack_ttl_hours: int = Field(
         default=48,
         ge=0,

roboco/runtime/orchestrator.py

@@ -933,6 +933,7 @@ class AgentOrchestrator:
         instance._instances = {}
         instance._last_audit_spawn_at = None
         instance._notification_spawn_at = {}
+        instance._notification_spawn_count = {}
         return instance
 
     def __init__(
@@ -1060,6 +1061,11 @@ class AgentOrchestrator:
         # retries if it is still unacked. In-memory by design (a restart just
         # allows one immediate retry — a tick damper, not durable state).
         self._notification_spawn_at: dict[tuple[str, str], float] = {}
+        # Hard-cap counter companion to _notification_spawn_at: spawns per
+        # (agent, notification) so a wedged escalation stops respawning its
+        # target past notification_spawn_max_attempts (the no-task_id analogue
+        # of the PM respawn breaker). In-memory (a restart allows a fresh run).
+        self._notification_spawn_count: dict[tuple[str, str], int] = {}
         # Cluster C5: a board review is a two-reviewer gate — BOTH the Product
         # Owner and the Head of Marketing must review a board/coordination task
         # before it is handed to the CEO for Approve & Start. Once both have
@@ -4486,10 +4492,19 @@ class AgentOrchestrator:
     def _notification_spawn_cooled(
         self, agent_slug: str, notification_id: str | None
     ) -> bool:
-        """True when this (agent, notification) spawned within the cooldown.
+        """True when this (agent, notification) spawn is suppressed.
 
-        Returns False — and stamps the pair — when a spawn is allowed. A
-        notification with no id is never damped (fail-open: better one extra
+        Two guards: the cross-tick cooldown (one spawn per window), AND a hard
+        cap — past ``notification_spawn_max_attempts`` spawns for the same
+        notification without it being acknowledged, stop re-spawning entirely.
+        Without the cap a wedged escalation/alert whose recipient never resolves
+        it re-spawns that recipient every window forever (these dispatchers
+        carry no task_id, so the PM respawn breaker never sees them). The cap is
+        id-scoped: a fresh escalation (new notification id) is unaffected, and a
+        resolved one stops being fetched and prunes out.
+
+        Returns False — and stamps + counts the pair — when a spawn is allowed.
+        A notification with no id is never damped (fail-open: better one extra
         spawn than a silently dropped escalation).
         """
         if not notification_id:
@@ -4499,20 +4514,116 @@ class AgentOrchestrator:
         store: dict[tuple[str, str], float] = self.__dict__.setdefault(
             "_notification_spawn_at", {}
         )
+        counts: dict[tuple[str, str], int] = self.__dict__.setdefault(
+            "_notification_spawn_count", {}
+        )
         key = (agent_slug, str(notification_id))
         now = time.monotonic()
         cooldown = settings.notification_spawn_cooldown_seconds
         last = store.get(key)
         if last is not None and (now - last) < cooldown:
             return True
+        if self._notification_spawn_over_cap(key, store, counts, now):
+            return True
+        counts[key] = counts.get(key, 0) + 1
         store[key] = now
         if len(store) > self._NOTIFICATION_COOLDOWN_PRUNE_AT:
-            cutoff = now - cooldown
-            self._notification_spawn_at = {
-                k: v for k, v in store.items() if v >= cutoff
-            }
+            self._prune_notification_spawn_maps(now - cooldown)
         return False
 
+    def _notification_spawn_over_cap(
+        self,
+        key: tuple[str, str],
+        store: dict[tuple[str, str], float],
+        counts: dict[tuple[str, str], int],
+        now: float,
+    ) -> bool:
+        """True (and suppresses the spawn) once ``key`` has spawned
+        ``notification_spawn_max_attempts`` times without the notification being
+        acknowledged — the no-task_id analogue of the PM respawn breaker.
+        Re-stamps so the capped entry survives pruning (which would otherwise
+        drop the count and reset the cap); logs exactly once at the trip.
+        """
+        max_attempts = settings.notification_spawn_max_attempts
+        attempts = counts.get(key, 0)
+        if not (max_attempts and attempts >= max_attempts):
+            return False
+        store[key] = now
+        if attempts == max_attempts:
+            counts[key] = attempts + 1
+            logger.warning(
+                "escalation respawn loop broken — a notification kept "
+                "respawning its target without being acknowledged; "
+                "suppressing further spawns until it is resolved",
+                agent_slug=key[0],
+                notification_id=key[1],
+                attempts=attempts,
+                max_attempts=max_attempts,
+            )
+        return True
+
+    def _prune_notification_spawn_maps(self, cutoff: float) -> None:
+        """Drop cooldown/count entries stamped before ``cutoff`` (both maps
+        stay aligned so a surviving cap keeps its count)."""
+        self._notification_spawn_at = {
+            k: v for k, v in self._notification_spawn_at.items() if v >= cutoff
+        }
+        survivors = set(self._notification_spawn_at)
+        self._notification_spawn_count = {
+            k: v for k, v in self._notification_spawn_count.items() if k in survivors
+        }
+
+    @staticmethod
+    def _parse_iso_dt(value: Any) -> datetime | None:
+        """Parse a notification's ISO timestamp to an aware UTC datetime, or
+        None if absent/unparseable. Naive values are assumed UTC."""
+        if not value:
+            return None
+        try:
+            dt = datetime.fromisoformat(str(value).replace("Z", "+00:00"))
+        except ValueError:
+            return None
+        return dt if dt.tzinfo else dt.replace(tzinfo=UTC)
+
+    async def _fetch_task_status(
+        self, client: httpx.AsyncClient, task_id: str
+    ) -> str | None:
+        """Best-effort GET /tasks/{id} → status string (None on any failure —
+        fail-open so a fetch hiccup never suppresses a real escalation)."""
+        try:
+            resp = await client.get(f"{self._api_url}/tasks/{task_id}")
+            if resp.status_code == http_status.HTTP_200_OK:
+                status = resp.json().get("status")
+                return str(status) if status is not None else None
+        except Exception as exc:
+            logger.debug("notification task-status fetch failed", error=str(exc))
+        return None
+
+    async def _notification_has_live_work(
+        self, client: httpx.AsyncClient, notif: dict[str, Any]
+    ) -> bool:
+        """False when a notification has no live work behind it — the 'is there
+        actually something to do' gate for notification-triggered spawns. Three
+        obvious markers: it has expired, it is stale past the spawn-age window
+        (wedged / reloaded from before a restart), or its related task is
+        already terminal (the work is done). Fail-open: an unparseable field or
+        a failed task fetch never suppresses a spawn.
+        """
+        now = datetime.now(UTC)
+        expires = self._parse_iso_dt(notif.get("expires_at"))
+        if expires is not None and expires <= now:
+            return False
+        max_age = settings.notification_spawn_max_age_seconds
+        ts = self._parse_iso_dt(notif.get("timestamp"))
+        if max_age and ts is not None and (now - ts).total_seconds() > max_age:
+            return False
+        task_id = notif.get("related_task_id")
+        if task_id:
+            status = await self._fetch_task_status(client, str(task_id))
+            if status in ("completed", "cancelled"):
+                return False
+        return True
+
     def _is_parallel_phase_claim(
         self, task: dict[str, Any], dev_uuid: str | None
     ) -> bool:
@@ -13722,6 +13833,8 @@ Never `commit`, never write code, never run `git`. PMs coordinate.
 
                 if self._notification_spawn_cooled(agent_slug, notif.get("id")):
                     continue
+                if not await self._notification_has_live_work(client, notif):
+                    continue
                 await self.spawn_agent(
                     agent_id=agent_slug,
                     initial_prompt=self._build_escalation_prompt(notif),
@@ -13753,6 +13866,8 @@ Never `commit`, never write code, never run `git`. PMs coordinate.
 
                 if self._notification_spawn_cooled(agent_slug, notif.get("id")):
                     continue
+                if not await self._notification_has_live_work(client, notif):
+                    continue
                 await self.spawn_agent(
                     agent_id=agent_slug,
                     initial_prompt=self._build_approval_prompt(notif),

tests/unit/runtime/test_notification_live_work.py

@@ -0,0 +1,98 @@
+"""The 'is there actually live work' gate for notification-triggered spawns.
+
+Escalation/approval dispatchers must not revive an agent for a notification
+that has expired, is stale past the spawn-age window, or whose related task is
+already terminal — otherwise a wedged/old notification loops the fleet.
+"""
+
+from __future__ import annotations
+
+from datetime import UTC, datetime, timedelta
+from typing import Any
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+from roboco.config import settings
+from roboco.runtime.orchestrator import AgentOrchestrator
+
+
+def _orch() -> AgentOrchestrator:
+    # _api_url is a read-only property (settings.internal_api_url); the mock
+    # client below ignores the URL, so no wiring is needed.
+    return AgentOrchestrator.__new__(AgentOrchestrator)
+
+
+def _client(task_status: str | None = None, *, fail: bool = False) -> Any:
+    client = MagicMock()
+    if fail:
+        client.get = AsyncMock(side_effect=RuntimeError("boom"))
+        return client
+    resp = MagicMock()
+    resp.status_code = 200
+    resp.json = MagicMock(return_value={"status": task_status})
+    client.get = AsyncMock(return_value=resp)
+    return client
+
+
+def _iso(dt: datetime) -> str:
+    return dt.isoformat()
+
+
+@pytest.mark.asyncio
+async def test_expired_notification_has_no_work() -> None:
+    orch = _orch()
+    notif = {"expires_at": _iso(datetime.now(UTC) - timedelta(minutes=1))}
+    assert await orch._notification_has_live_work(_client(), notif) is False
+
+
+@pytest.mark.asyncio
+async def test_stale_notification_has_no_work() -> None:
+    orch = _orch()
+    old = datetime.now(UTC) - timedelta(
+        seconds=settings.notification_spawn_max_age_seconds + 60
+    )
+    notif = {"timestamp": _iso(old)}
+    assert await orch._notification_has_live_work(_client(), notif) is False
+
+
+@pytest.mark.asyncio
+async def test_terminal_related_task_has_no_work() -> None:
+    orch = _orch()
+    notif = {"timestamp": _iso(datetime.now(UTC)), "related_task_id": "t1"}
+    assert await orch._notification_has_live_work(_client("completed"), notif) is False
+    assert await orch._notification_has_live_work(_client("cancelled"), notif) is False
+
+
+@pytest.mark.asyncio
+async def test_fresh_notification_with_live_task_has_work() -> None:
+    orch = _orch()
+    notif = {"timestamp": _iso(datetime.now(UTC)), "related_task_id": "t1"}
+    assert await orch._notification_has_live_work(_client("in_progress"), notif) is True
+
+
+@pytest.mark.asyncio
+async def test_fresh_notification_no_task_has_work() -> None:
+    orch = _orch()
+    notif = {"timestamp": _iso(datetime.now(UTC))}
+    assert await orch._notification_has_live_work(_client(), notif) is True
+
+
+@pytest.mark.asyncio
+async def test_fail_open_on_fetch_error_and_bad_timestamp() -> None:
+    orch = _orch()
+    # A failed task fetch must not suppress a real escalation.
+    notif = {"timestamp": _iso(datetime.now(UTC)), "related_task_id": "t1"}
+    assert await orch._notification_has_live_work(_client(fail=True), notif) is True
+    # An unparseable timestamp is ignored (no false-stale), not treated as old.
+    assert (
+        await orch._notification_has_live_work(_client(), {"timestamp": "nope"}) is True
+    )
+
+
+@pytest.mark.asyncio
+async def test_staleness_gate_disabled_when_zero() -> None:
+    orch = _orch()
+    ancient = datetime.now(UTC) - timedelta(days=30)
+    notif = {"timestamp": _iso(ancient)}
+    with patch.object(settings, "notification_spawn_max_age_seconds", 0):
+        assert await orch._notification_has_live_work(_client(), notif) is True

tests/unit/runtime/test_notification_spawn_cooldown.py

@@ -59,6 +59,67 @@ def test_missing_notification_id_never_damped() -> None:
         assert orch._notification_spawn_at == {}
 
 
+def test_hard_cap_breaks_respawn_loop() -> None:
+    """Past max_attempts spawns for one unacked notification, the target is
+    suppressed forever — the loop breaker for no-task_id escalations."""
+    orch = _orch()
+    with (
+        patch.object(settings, "notification_spawn_cooldown_seconds", 600),
+        patch.object(settings, "notification_spawn_max_attempts", 3),
+        patch("roboco.runtime.orchestrator.time.monotonic") as clock,
+    ):
+        # Each retry lands in a fresh cooldown window (advance past 600s).
+        for i in range(3):
+            clock.return_value = 1_000.0 + i * 700
+            assert orch._notification_spawn_cooled("fe-pm", "stuck") is False
+        # 4th+ window: cap tripped — suppressed despite the cooldown elapsing.
+        for i in range(3, 8):
+            clock.return_value = 1_000.0 + i * 700
+            assert orch._notification_spawn_cooled("fe-pm", "stuck") is True
+        # A different notification is unaffected by another's cap.
+        clock.return_value = 9_000.0
+        assert orch._notification_spawn_cooled("fe-pm", "other") is False
+
+
+def test_zero_max_attempts_disables_cap() -> None:
+    orch = _orch()
+    with (
+        patch.object(settings, "notification_spawn_cooldown_seconds", 600),
+        patch.object(settings, "notification_spawn_max_attempts", 0),
+        patch("roboco.runtime.orchestrator.time.monotonic") as clock,
+    ):
+        # Cap off: only the cooldown gates, every elapsed window respawns.
+        for i in range(20):
+            clock.return_value = 1_000.0 + i * 700
+            assert orch._notification_spawn_cooled("fe-pm", "stuck") is False
+
+
+def test_cap_survives_prune() -> None:
+    """A capped entry stays suppressed even after a prune sweep fires (the
+    prune must not drop the count and reset the cap)."""
+    orch = _orch()
+    prune_at = AgentOrchestrator._NOTIFICATION_COOLDOWN_PRUNE_AT
+    with (
+        patch.object(settings, "notification_spawn_cooldown_seconds", 600),
+        patch.object(settings, "notification_spawn_max_attempts", 2),
+        patch("roboco.runtime.orchestrator.time.monotonic") as clock,
+    ):
+        clock.return_value = 10_000.0
+        assert orch._notification_spawn_cooled("fe-pm", "stuck") is False
+        clock.return_value = 10_700.0
+        assert orch._notification_spawn_cooled("fe-pm", "stuck") is False
+        clock.return_value = 11_400.0
+        assert orch._notification_spawn_cooled("fe-pm", "stuck") is True  # capped
+        # Force a prune sweep with many fresh keys.
+        for i in range(prune_at + 1):
+            orch._notification_spawn_cooled("be-pm", f"n{i}")
+        # Advance well past the cooldown so only the surviving cap — not the
+        # cooldown — can suppress the next "stuck" check. A prune that dropped
+        # the count would reset the cap and allow a spawn (return False) here.
+        clock.return_value = 20_000.0
+        assert orch._notification_spawn_cooled("fe-pm", "stuck") is True
+
+
 def test_map_prunes_expired_entries() -> None:
     orch = _orch()
     prune_at = AgentOrchestrator._NOTIFICATION_COOLDOWN_PRUNE_AT