Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions docs/reference/agent-wait-patterns.md
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,46 @@ respondent. Messages like this are bus noise.
- `HANDOFF` — "I need you to act on this artifact"
- `STATUS` / `PROGRESS` — informational, no reply expected

### Anti-pattern 5 — Producer waits on `CONSENSUS_CONFIRMED` before its own confirm has succeeded (#2064)

```bash
# ❌ DO NOT DO THIS — happens when a producer treats the post-confirm
# STAY ALIVE wait_loop as the recovery path for a `pending_acks` confirm.
egg-orch consensus propose ...
egg-orch consensus confirmed # returns status='pending_acks'
# because another producer
# hasn't proposed yet
egg-orch message wait-loop \
--for CONSENSUS_CONFIRMED \ # ← circular: own confirm
--for CONSENSUS_RE_REVIEW \ # is part of what generates
--for OVERSEER_ALERT --timeout 60 # this signal globally
```

**Why it's wrong:** the global `CONSENSUS_CONFIRMED` signal only fires
when **every** agent — including this producer — has confirmed. Waiting
on it before the producer's own confirm has been accepted by the
tracker is a self-deadlock. Observed in pipeline `issue-1965`: the
documenter sat in this wait for ~36 minutes, woken only by the
overseer's `agent-heartbeat-stall` band-aid.

The orchestrator's `/messages/wait` endpoint now rejects this pattern
with **HTTP 400** when the caller's role is in producer state
`WORKING` or `PROPOSED` and the wait includes `CONSENSUS_CONFIRMED` —
the wrapper surfaces this as exit code 3 (permanent error). Read the
error: it tells you what to wait for instead.

**Fix:** the post-confirm STAY ALIVE wait is only legitimate **after**
your own confirm has succeeded (status `confirmed`, not `pending_acks`).
For a `pending_acks` recovery loop:

- **Global zero-proposal** (another producer hasn't proposed): wait on
`CONSENSUS_PROPOSE` (and `OVERSEER_ALERT`), then re-issue
`egg-orch consensus confirmed`.
- **Your reviewers haven't ACKed yet**: wait on `CONSENSUS_ACK,CONSENSUS_NACK`
per the producer-lifecycle Step 4 idiom, then re-issue confirm when
the orchestrator's directed STATUS nudge ("ready to confirm")
arrives.

## 3. Exit-Code Contract for `egg-orch message wait`

`egg-orch message wait` returns a deterministic exit code so the wrapper
Expand Down
15 changes: 15 additions & 0 deletions orchestrator/peer_consensus.py
Original file line number Diff line number Diff line change
Expand Up @@ -1245,6 +1245,21 @@ def get_state(self) -> dict[str, Any]:
"""Alias for evaluate() -- compatibility with ConsensusEvaluator."""
return self.evaluate()

def is_producer_pending_confirm(self, role: str) -> bool:
"""True if ``role`` is a producer that has not yet reached CONFIRMED.

Used by the ``/messages/wait`` endpoint to reject incoherent
``wait_loop --for CONSENSUS_CONFIRMED`` calls from producers
whose own confirm hasn't succeeded — their confirm is part of
what generates global consensus, so the wait would deadlock
(#2064). Reviewer-only roles return False (they may legitimately
wait on other agents' confirms).
"""
with self._lock:
if not self.graph.is_producer(role):
return False
return self._producer_phases.get(role) != ConsensusPhase.CONFIRMED

def are_all_producers_working(self, reviewer: str) -> bool:
"""Check if all upstream producers for a reviewer are still in WORKING phase.

Expand Down
67 changes: 67 additions & 0 deletions orchestrator/routes/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,69 @@ def _apply_delphi_filter(
return filtered_messages


# Message types that are produced *as a side effect* of a producer's
# own confirm reaching global consensus. A producer in WORKING/PROPOSED
# that waits on these would be waiting on itself — its own confirm is
# part of what generates the signal — and would deadlock until the
# overseer's stall detector intervened (#2064).
_PRODUCER_PENDING_CONFIRM_REJECTED_FOR_TYPES: frozenset[str] = frozenset({"CONSENSUS_CONFIRMED"})


def _check_producer_pending_confirm_guard(
pipeline_id: str,
role: str | None,
wait_for_types: list[str],
) -> tuple[Response, int] | None:
"""Reject ``wait`` calls where a non-confirmed producer waits on
``CONSENSUS_CONFIRMED``.

A producer's own ``mcp__brc__confirm`` is part of what generates
the global ``CONSENSUS_CONFIRMED`` signal, so a producer in
``WORKING`` or ``PROPOSED`` that blocks on it would wait on
itself (#2064). Rather than letting the overseer's heartbeat-stall
detector bail the pipeline out minutes later, we surface the bug
immediately with an actionable error.

The guard intentionally ignores the route's ``from`` filter — even
a wait narrowly scoped to a peer's per-agent ``CONSENSUS_CONFIRMED``
is still part of a chain that requires this producer's own confirm
to fire first. No documented producer pattern waits this way while
in ``WORKING``/``PROPOSED``, so the over-rejection is harmless; any
future cross-producer sync that wants to bypass it should update
both this guard and the wait_loop client contract.

Returns ``None`` when the wait should proceed; otherwise an error
response tuple ready to return from the route.
"""
if not role:
return None
blocking = _PRODUCER_PENDING_CONFIRM_REJECTED_FOR_TYPES.intersection(wait_for_types)
if not blocking:
return None
try:
from peer_consensus import get_peer_consensus_tracker
except ImportError:
get_peer_consensus_tracker = None # type: ignore[assignment]
if not get_peer_consensus_tracker:
return None
tracker = get_peer_consensus_tracker(pipeline_id)
if tracker is None:
return None
if not tracker.is_producer_pending_confirm(role):
return None
sorted_blocking = sorted(blocking)
return _make_error(
f"Producer '{role}' cannot wait on {sorted_blocking} before its own "
"consensus_confirmed has succeeded — its own confirm is part of "
"what generates that signal, so the wait would deadlock (#2064). "
"Call mcp__brc__confirm first; if it returns status='pending_acks' "
"(e.g. another producer hasn't proposed yet, or your reviewers "
"haven't ACKed), wait on the prerequisite events instead — "
"CONSENSUS_PROPOSE from missing producers, CONSENSUS_ACK from "
"your reviewers, or CONSENSUS_RE_REVIEW — then retry confirm."
)


@messages_bp.route("/<pipeline_id>/messages/wait", methods=["GET"])
def wait_messages(pipeline_id: str) -> tuple[Response, int]:
"""Block on a typed message event.
Expand Down Expand Up @@ -405,6 +468,10 @@ def wait_messages(pipeline_id: str) -> tuple[Response, int]:
# so the caller actually observes blocking semantics.
timeout = 1

guard_response = _check_producer_pending_confirm_guard(pipeline_id, role, wait_for_types)
if guard_response is not None:
return guard_response

message_store = get_message_store()

_track_long_poll_start()
Expand Down
Loading
Loading