Skip to content

Commit 6181d72

Browse files
lwgrayclaude
andcommitted
fix(#667): circuit breaker — BLOCK task after N consecutive silent recoveries
Fix 3a from issue #667 (the board-policy half; the spawn-backoff half 3b is intentionally NOT built — the spawn controller is deleted by the session-model migration, epic #706). A "silent" recovery is a lease recovery where the agent never reported progress (renewal_count == 0, the `updates=0` signature of the snake_game runaway: 81 registrations over 3+ hours). After max_silent_recoveries (default 3) consecutive silent recoveries, recover_expired_lease now marks the task BLOCKED instead of returning it to TODO: BLOCKED tasks are never offered by request_next_task, so the reclaim/re-assign loop physically cannot continue — regardless of who supplies agents (Invariant #1: the guard lives in Marcus, not the spawner). - A recovery WITH progress resets the streak (slow-but-alive is not the silent-failure pattern — the #703 lesson at board level) - BLOCKED path performs the same coordination cleanup as normal recovery (lease, persistence, server callback) — no #485-class leaks - Diagnostic kanban comment + task_blocked_circuit_breaker history event; human resets to TODO to retry (streak starts fresh) 8 new tests; full lease module 74/74; mypy + pre-commit clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 171435f commit 6181d72

2 files changed

Lines changed: 363 additions & 0 deletions

File tree

src/core/assignment_lease.py

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ def __init__(
220220
enable_adaptive_leases: bool = True,
221221
task_list: Optional[List[Task]] = None,
222222
silence_multiplier: float = 5.0,
223+
max_silent_recoveries: int = 3,
223224
):
224225
"""
225226
Initialize the lease manager.
@@ -254,6 +255,14 @@ def __init__(
254255
Enable smart lease duration adjustments.
255256
task_list
256257
Optional reference to project tasks for recovery info updates.
258+
max_silent_recoveries
259+
Circuit breaker (issue #667 Fix 3a): number of CONSECUTIVE
260+
silent recoveries (lease expired with zero progress updates,
261+
``renewal_count == 0``) after which the task is marked
262+
BLOCKED instead of returned to TODO. Stops the
263+
reclaim/re-assign runaway loop regardless of who supplies
264+
agents. A recovery where the agent reported progress resets
265+
the streak.
257266
"""
258267
self.kanban_client = kanban_client
259268
self.assignment_persistence = assignment_persistence
@@ -282,6 +291,14 @@ def __init__(
282291
self.stuck_task_threshold_renewals = stuck_task_threshold_renewals
283292
self.enable_adaptive_leases = enable_adaptive_leases
284293
self.silence_multiplier = silence_multiplier
294+
self.max_silent_recoveries = max_silent_recoveries
295+
296+
# Circuit breaker state (issue #667 Fix 3a): consecutive SILENT
297+
# recovery count per task. In-memory by design — the observed
298+
# runaway (81 registrations over 3+ hours) happened inside one
299+
# server process. Restart-durable counters ride with the
300+
# session-model registration-persistence work (epic #706).
301+
self._silent_recovery_streaks: Dict[str, int] = {}
285302

286303
# Active leases tracked in memory
287304
self.active_leases: Dict[str, AssignmentLease] = {}
@@ -1037,6 +1054,27 @@ async def recover_expired_lease(self, lease: AssignmentLease) -> bool:
10371054
f"(expired: {lease.lease_expires.isoformat()})"
10381055
)
10391056

1057+
# Circuit breaker (issue #667 Fix 3a): a SILENT recovery is one
1058+
# where the agent never reported progress (renewal_count == 0 —
1059+
# the `updates=0` signature of the snake_game runaway). After
1060+
# max_silent_recoveries consecutive silent recoveries, mark the
1061+
# task BLOCKED instead of returning it to TODO: the failure is
1062+
# almost certainly external (LLM/API outage, broken env), and
1063+
# every fresh agent will hit it identically. BLOCKED tasks are
1064+
# never offered by request_next_task, so the loop cannot
1065+
# continue — regardless of who is supplying agents
1066+
# (Invariant #1: the guard must live in Marcus, not a spawner).
1067+
if lease.renewal_count == 0:
1068+
streak = self._silent_recovery_streaks.get(lease.task_id, 0) + 1
1069+
self._silent_recovery_streaks[lease.task_id] = streak
1070+
if streak >= self.max_silent_recoveries:
1071+
return await self._block_task_after_silent_recoveries(lease, streak)
1072+
else:
1073+
# An agent that reported progress but still expired is a
1074+
# slow-but-alive pattern (see #703), not the silent-failure
1075+
# pattern — reset the streak.
1076+
self._silent_recovery_streaks.pop(lease.task_id, None)
1077+
10401078
# Calculate time spent
10411079
now = datetime.now(timezone.utc)
10421080
time_spent = now - lease.assigned_at
@@ -1217,6 +1255,122 @@ async def recover_expired_lease(self, lease: AssignmentLease) -> bool:
12171255
logger.error(f"Error recovering lease for task {lease.task_id}: {e}")
12181256
return False
12191257

1258+
async def _block_task_after_silent_recoveries(
1259+
self, lease: AssignmentLease, streak: int
1260+
) -> bool:
1261+
"""Mark a task BLOCKED after N consecutive silent recoveries.
1262+
1263+
Terminal arm of the circuit breaker (issue #667 Fix 3a). Instead
1264+
of resetting the task to TODO (which would let yet another agent
1265+
claim it and fail identically), flip it to BLOCKED, surface a
1266+
diagnostic comment on the board, and perform the same
1267+
coordination-state cleanup as a normal recovery so no in-memory
1268+
agent state leaks (the issue #485 failure class).
1269+
1270+
A human (or a later automated policy) can reset the task to TODO
1271+
to retry; the streak counter is cleared here so a reset starts
1272+
fresh.
1273+
1274+
Parameters
1275+
----------
1276+
lease : AssignmentLease
1277+
The expired lease whose recovery tripped the breaker.
1278+
streak : int
1279+
The consecutive-silent-recovery count that tripped it.
1280+
1281+
Returns
1282+
-------
1283+
bool
1284+
True if the BLOCKED transition was applied (cleanup errors on
1285+
non-authoritative writes are logged, not raised).
1286+
"""
1287+
logger.warning(
1288+
f"Circuit breaker: task {lease.task_id} has been recovered "
1289+
f"{streak} consecutive times with zero progress updates "
1290+
f"(last agent: {lease.agent_id}). Marking BLOCKED instead of "
1291+
f"TODO — likely external failure (LLM/API outage, broken "
1292+
f"environment). Human investigation required."
1293+
)
1294+
1295+
# 1. Update task model (source of truth)
1296+
task = self._find_task(lease.task_id)
1297+
if task:
1298+
task.status = TaskStatus.BLOCKED
1299+
task.assigned_to = None
1300+
1301+
# 2. Remove from active leases
1302+
async with self.lease_lock:
1303+
if lease.task_id in self.active_leases:
1304+
del self.active_leases[lease.task_id]
1305+
1306+
# 3. Remove assignment from persistence
1307+
await self.assignment_persistence.remove_assignment(lease.agent_id)
1308+
1309+
# 4. Invoke recovery callback so server cleans in-memory state
1310+
if self.on_recovery_callback is not None:
1311+
try:
1312+
self.on_recovery_callback(lease.agent_id, lease.task_id)
1313+
except Exception as e:
1314+
logger.warning(
1315+
f"Recovery callback failed for task {lease.task_id}: {e}"
1316+
)
1317+
1318+
# 5. Board: status → BLOCKED, clear ownership
1319+
try:
1320+
if hasattr(self.kanban_client, "update_task"):
1321+
await self.kanban_client.update_task(
1322+
lease.task_id,
1323+
{"status": TaskStatus.BLOCKED, "assigned_to": None},
1324+
)
1325+
elif hasattr(self.kanban_client, "update_task_status"):
1326+
await self.kanban_client.update_task_status(
1327+
lease.task_id, TaskStatus.BLOCKED
1328+
)
1329+
except Exception as e:
1330+
logger.error(f"Failed to mark task {lease.task_id} BLOCKED on board: {e}")
1331+
1332+
# 6. Diagnostic comment (observability dual-write)
1333+
try:
1334+
await self.kanban_client.add_comment(
1335+
lease.task_id,
1336+
(
1337+
f"🛑 **CIRCUIT BREAKER — task BLOCKED by Marcus**\n\n"
1338+
f"{streak} consecutive agents claimed this task and "
1339+
f"reported no progress before their lease expired. "
1340+
f"This pattern almost always means an external "
1341+
f"failure every fresh agent hits identically — an "
1342+
f"LLM/API outage, a broken environment, or a task "
1343+
f"that cannot run as specified — so retrying with "
1344+
f"more agents only burns cost (issue #667).\n\n"
1345+
f"**What to do:** investigate the root cause, then "
1346+
f"reset this task's status to TODO to retry.\n\n"
1347+
f"- Last agent: {lease.agent_id}\n"
1348+
f"- Silent recoveries: {streak} "
1349+
f"(threshold: {self.max_silent_recoveries})\n"
1350+
),
1351+
)
1352+
except Exception as e:
1353+
logger.warning(
1354+
f"Failed to write circuit-breaker comment for " f"{lease.task_id}: {e}"
1355+
)
1356+
1357+
# 7. Auditable history event
1358+
self.lease_history.append(
1359+
{
1360+
"event": "task_blocked_circuit_breaker",
1361+
"task_id": lease.task_id,
1362+
"agent_id": lease.agent_id,
1363+
"timestamp": datetime.now(timezone.utc).isoformat(),
1364+
"consecutive_silent_recoveries": streak,
1365+
"threshold": self.max_silent_recoveries,
1366+
}
1367+
)
1368+
1369+
# 8. Clear the streak — a human reset to TODO starts fresh
1370+
self._silent_recovery_streaks.pop(lease.task_id, None)
1371+
1372+
return True
1373+
12201374
async def get_expiring_leases(self) -> List[AssignmentLease]:
12211375
"""
12221376
Get leases that are expiring soon.

0 commit comments

Comments
 (0)