mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(dispatch): stop burning breaker strikes on dependency-held blocked tasks; collision notify rides the delegate transaction (#726)
* fix(dispatch): stop burning breaker strikes on dependency-held blocked tasks; collision notify rides the delegate transaction Two follow-ups from the 2026-07-29 blocked-task wave: - _dispatch_blocker_work now skips a blocked task whose dependencies are non-terminal BEFORE the respawn gate. spawn_agent's readiness gate was refusing these anyway, but each refused attempt burned a respawn- breaker strike and, once tripped at 4, a duplicate CEO escalation every tick (seen live on the eslint-audit task held behind a paused backend dependency). The dispatcher re-checks each tick and proceeds the moment the dependency goes terminal — same resume path as before, minus the noise. - send_collision_sequencing_notification gains the db_session threading its sibling senders already have, and TaskService passes its own session. delegate wires collision edges inside a transaction whose held-back child row is not yet committed; the notification INSERT ran on a separate auto-commit connection and died on the related_task_id FK (ForeignKeyViolationError seen live in cell_pm/delegate), silently losing the coordination alert. Riding the caller's transaction makes the row visible and commits both atomically. * test(notification): cast the fake session for the gate's tests-scope mypy --------- Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
@@ -15485,6 +15485,15 @@ Never `commit`, never write code, never run `git`. PMs coordinate.
|
|||||||
if self._is_hitl_blocked(task):
|
if self._is_hitl_blocked(task):
|
||||||
continue
|
continue
|
||||||
|
|
||||||
|
# A dependency-held blocked task can't be unblocked by any
|
||||||
|
# resolver until the dependency lands — spawn_agent's readiness
|
||||||
|
# gate would refuse anyway, and each refused attempt burned a
|
||||||
|
# respawn-breaker strike + a CEO escalation once tripped
|
||||||
|
# (2026-07-29). Skip quietly; this dispatcher re-checks every
|
||||||
|
# tick and proceeds once the dependency goes terminal.
|
||||||
|
if await self._check_dependencies_terminal(client, task):
|
||||||
|
continue
|
||||||
|
|
||||||
agent_id = self._blocker_resolver_slug(task)
|
agent_id = self._blocker_resolver_slug(task)
|
||||||
if not agent_id:
|
if not agent_id:
|
||||||
continue
|
continue
|
||||||
|
|||||||
@@ -524,12 +524,16 @@ class NotificationService:
|
|||||||
to_ceo: str = "ceo",
|
to_ceo: str = "ceo",
|
||||||
held_back_title: str | None = None,
|
held_back_title: str | None = None,
|
||||||
blocking_title: str | None = None,
|
blocking_title: str | None = None,
|
||||||
|
db_session: AsyncSession | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Tell the held-back task's owner (+ CEO) it now waits on a sibling.
|
"""Tell the held-back task's owner (+ CEO) it now waits on a sibling.
|
||||||
|
|
||||||
Fired only for a newly-created collision-sequencing edge (see
|
Fired only for a newly-created collision-sequencing edge (see
|
||||||
``wire_sibling_collision_dag`` — a repeat wiring pass over an
|
``wire_sibling_collision_dag`` — a repeat wiring pass over an
|
||||||
already-wired pair contributes no edge, so this cannot double-fire).
|
already-wired pair contributes no edge, so this cannot double-fire).
|
||||||
|
``db_session`` must be the edge-wiring transaction's own session: the
|
||||||
|
held-back task row may be uncommitted there, and the FK on
|
||||||
|
``related_task_id`` fails on any other connection (2026-07-29).
|
||||||
"""
|
"""
|
||||||
recipients = list(dict.fromkeys(r for r in (held_back_assignee, to_ceo) if r))
|
recipients = list(dict.fromkeys(r for r in (held_back_assignee, to_ceo) if r))
|
||||||
if not recipients:
|
if not recipients:
|
||||||
@@ -556,7 +560,8 @@ class NotificationService:
|
|||||||
subject=f"Task {held_back_display} sequenced behind a sibling",
|
subject=f"Task {held_back_display} sequenced behind a sibling",
|
||||||
body=body,
|
body=body,
|
||||||
related_task_id=held_back_task_id,
|
related_task_id=held_back_task_id,
|
||||||
)
|
),
|
||||||
|
db_session,
|
||||||
)
|
)
|
||||||
|
|
||||||
async def send_unblock_notification(
|
async def send_unblock_notification(
|
||||||
|
|||||||
@@ -8944,6 +8944,7 @@ class TaskService(BaseService):
|
|||||||
held_back_assignee=str(owner) if owner is not None else None,
|
held_back_assignee=str(owner) if owner is not None else None,
|
||||||
held_back_title=held_back_title,
|
held_back_title=held_back_title,
|
||||||
blocking_title=blocking_title,
|
blocking_title=blocking_title,
|
||||||
|
db_session=self.session,
|
||||||
)
|
)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
self.log.warning("Collision-sequencing notify failed", error=str(e))
|
self.log.warning("Collision-sequencing notify failed", error=str(e))
|
||||||
|
|||||||
@@ -156,6 +156,40 @@ async def test_dispatch_blocker_work_spawns_non_hitl_blocked_task(
|
|||||||
spawn.assert_awaited_once()
|
spawn.assert_awaited_once()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_dispatch_blocker_work_skips_dependency_held_task(
|
||||||
|
monkeypatch: pytest.MonkeyPatch,
|
||||||
|
) -> None:
|
||||||
|
"""2026-07-29: a blocked task waiting on a non-terminal dependency must
|
||||||
|
be skipped BEFORE the respawn gate — spawn_agent's readiness gate was
|
||||||
|
going to refuse anyway, and each refused attempt burned a breaker strike
|
||||||
|
plus, once tripped, a CEO escalation notification."""
|
||||||
|
orch = _orch()
|
||||||
|
task: dict[str, Any] = {
|
||||||
|
"id": "t1",
|
||||||
|
"status": "blocked",
|
||||||
|
"blocker_resolver_type": None,
|
||||||
|
"team": "backend",
|
||||||
|
"assigned_to": AGENT_UUIDS["be-pm"],
|
||||||
|
"dependency_ids": ["d1"],
|
||||||
|
}
|
||||||
|
monkeypatch.setattr(orch, "_fetch_tasks", AsyncMock(return_value=[task]))
|
||||||
|
monkeypatch.setattr(
|
||||||
|
orch,
|
||||||
|
"_check_dependencies_terminal",
|
||||||
|
AsyncMock(return_value="Task t1 waiting on non-terminal dependency d1"),
|
||||||
|
)
|
||||||
|
gate = AsyncMock(return_value=False)
|
||||||
|
monkeypatch.setattr(orch, "_pm_respawn_should_gate", gate)
|
||||||
|
spawn = AsyncMock()
|
||||||
|
monkeypatch.setattr(orch, "spawn_agent", spawn)
|
||||||
|
|
||||||
|
await orch._dispatch_blocker_work(client=MagicMock())
|
||||||
|
|
||||||
|
spawn.assert_not_awaited()
|
||||||
|
gate.assert_not_awaited()
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# _claimed_task_needs_agent — claimed-but-no-agent detection
|
# _claimed_task_needs_agent — claimed-but-no-agent detection
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
@@ -544,6 +544,25 @@ async def test_send_collision_sequencing_notification(
|
|||||||
assert any("sequenced behind" in r.subject for r in rows)
|
assert any("sequenced behind" in r.subject for r in rows)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_send_collision_sequencing_notification_rides_caller_session(
|
||||||
|
svc: NotificationService,
|
||||||
|
) -> None:
|
||||||
|
"""2026-07-29: delegate wires collision edges inside a transaction whose
|
||||||
|
held-back task row is not yet committed — the notification INSERT must
|
||||||
|
ride that same session, or its related_task_id FK fails on any other
|
||||||
|
connection. No _patch_db_context here: opening a separate connection
|
||||||
|
would hit the real engine and fail the test."""
|
||||||
|
db = _FakeDb(agent_uuid=uuid4())
|
||||||
|
await svc.send_collision_sequencing_notification(
|
||||||
|
held_back_task_id="t2",
|
||||||
|
blocking_task_id="t1",
|
||||||
|
held_back_assignee="be-dev-1",
|
||||||
|
db_session=cast("Any", db),
|
||||||
|
)
|
||||||
|
assert [r for r in db.added if r.related_task_id == "t2"]
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_send_unblock_notification(svc: NotificationService) -> None:
|
async def test_send_unblock_notification(svc: NotificationService) -> None:
|
||||||
aid = uuid4()
|
aid = uuid4()
|
||||||
|
|||||||
@@ -618,6 +618,23 @@ async def test_wire_sibling_collision_dag_notifies_only_for_new_edges() -> None:
|
|||||||
assert add_dep_mock.await_count == wiring_passes
|
assert add_dep_mock.await_count == wiring_passes
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_collision_sequencing_notify_rides_task_service_session() -> None:
|
||||||
|
"""2026-07-29: the held-back task row is uncommitted in this transaction —
|
||||||
|
the notification must ride the SAME session or its related_task_id FK
|
||||||
|
fails (ForeignKeyViolationError seen live in cell_pm/delegate)."""
|
||||||
|
session = MagicMock(flush=AsyncMock())
|
||||||
|
svc = TaskService(session)
|
||||||
|
mock_ns = MagicMock()
|
||||||
|
mock_ns.send_collision_sequencing_notification = AsyncMock()
|
||||||
|
with patch(
|
||||||
|
"roboco.services.notification.NotificationService", return_value=mock_ns
|
||||||
|
):
|
||||||
|
await svc._notify_collision_sequencing(uuid4(), uuid4(), None)
|
||||||
|
kwargs = mock_ns.send_collision_sequencing_notification.await_args.kwargs
|
||||||
|
assert kwargs["db_session"] is session
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_mark_agent_idle_sets_status_idle() -> None:
|
async def test_mark_agent_idle_sets_status_idle() -> None:
|
||||||
agent = MagicMock(id=uuid4(), status=AgentStatus.ACTIVE, current_task_id=uuid4())
|
agent = MagicMock(id=uuid4(), status=AgentStatus.ACTIVE, current_task_id=uuid4())
|
||||||
|
|||||||
Reference in New Issue
Block a user