Fix agent circular-dependency deadlocks in agent-to-agent messaging and task delegatio - #581
Closed
QuanCheng-QC wants to merge 5 commits into
Closed
Fix agent circular-dependency deadlocks in agent-to-agent messaging and task delegatio#581QuanCheng-QC wants to merge 5 commits into
QuanCheng-QC wants to merge 5 commits into
Conversation
Two agents that block on each other inside react() stall until their timeouts, and replies could be lost or misclassified. This commit governs the blocking wait primitives - register reply waiters before sending in send_and_wait, post_and_wait and Workspace.channels, closing the lost-wakeup race where a fast reply arrived before the waiter existed - correlate replies precisely via the existing Event.response_to field. The messaging mod now propagates response_to and requires_response across the direct-message relay and exposes message_id so receivers can reply with correlation. A counter-request from the peer is no longer mistaken for a reply - add AgentClient.expect_event and EventWaiter as the register-then-send primitive, and implement the previously missing AgentClient.wait_mod_message that channel wait helpers were calling (they always raised AttributeError and silently returned None) - match channel wait conditions against the notification events the messaging mod actually emits (thread.reply.notification, thread.channel_message.notification, thread.reaction.notification) - mark react() execution via a ContextVar in the runner and warn with DeprecationWarning when a blocking wait primitive is called inside react(). Setting OPENAGENTS_STRICT_NO_BLOCKING_WAIT=1 upgrades the warning to BlockingWaitInReactError. Agent-to-mod RPCs are unaffected - add WorkerAgent.reply_direct for correlated replies to direct messages Covered by tests/workspace/test_sync_wait_governance.py
Task delegation had no defense against circular delegation. A->B->A (or any longer cycle) created tasks forever, each burning its own timeout while the loop kept spawning new work. Delegation lineage (task_delegation mod) - clients may pass parent_task_id when delegating as part of handling a task. The server derives root_task_id, delegation_chain and delegation_depth from the parent task in the task store, so a client cannot forge or truncate the chain - rejects self-delegation (agent ids normalized so agent prefix aliases cannot bypass the check), delegation cycles, chains beyond max_delegation_depth, and delegators over the max_active_delegations_per_agent fuse, since cycle checks alone cannot stop non-cyclic exponential fan-out - both creation paths are covered, direct task.delegate and capability-routed tasks - assignment notifications carry the lineage so assignees can extend the chain with parent_task_id on their own delegations Wait-graph deadlock detection (event gateway) - every event flows through the central gateway, so a WaitGraphMonitor builds a wait-for graph there from requires_response requests and clears edges on matching response_to replies, with a TTL for abandoned waits - when an edge closes a cycle, every agent on it receives an agent.wait.deadlock_detected event naming the cycle. send_and_wait recognizes reports about its own request and fails fast with a clear log instead of blocking until timeout - this covers what lineage checks cannot see, mutual send_and_wait, independent tasks pointing at each other, and mixed-mechanism cycles Covered by tests/mods/test_task_delegation.py (TestDelegationLineage), tests/network/test_wait_graph.py and tests/workspace/test_sync_wait_governance.py
- task.complete responses reported completed_at as None because the delegation snapshot was taken before the completion timestamp was written; re-extract after the update - a locally timed-out external task kept its background polling loop alive until process shutdown; stop polling when the timeout marks the task failed - gRPC SendEvent had no deadline, so a stalled server blocked the caller forever; use a 30s deadline in line with the client wait defaults - document the no-blocking-wait-in-react rule and the continuation pattern with reply_direct in the workspace interface guide
Four merge blockers from review plus two follow-up suggestions - only explicit blocking waits become wait-graph edges. send_and_wait now marks its request with blocking_wait and wait_timeout metadata; requires_response alone no longer creates an edge, so continuation and fire-and-forget requests cannot form false cycles that abort real waiters. Edges expire at the waiter's own deadline instead of a fixed TTL - deadlock notifications bypass event subscription filtering in the gateway; an agent subscribed only to thread.* still receives agent.wait.deadlock_detected instead of silently degrading to a blind timeout - wait-graph endpoints are canonicalized (agent prefix stripped) so alias forms join one graph, and a self-wait is reported as an immediate single-node cycle instead of being ignored - delegation lineage validation and task creation now run under one lock, so concurrent delegations cannot both pass the active-delegation fuse before either task is stored - DFS returns the exact edges forming the cycle and only those request ids are reported, leaving unrelated concurrent waits between the same agents untouched - react scope is bound to its owning asyncio task; background tasks spawned inside react() inherit the ContextVar but are no longer treated as inside react, so post-handler continuations cannot trigger false warnings or strict-mode errors Adds regression tests for all of the above and for the earlier hygiene fixes (completed_at now populated, external polling stopped on local timeout, gRPC SendEvent deadline). Rebased onto latest develop.
Second-round review blockers - the gRPC Event proto had no requires_response / response_to fields, so both were dropped on ingress. Precise reply matching fell back to the legacy source-only heuristic and wait-graph edges were never cleared by responses from gRPC agents. Added the fields to the proto (backward-compatible tag numbers 11 and 12), regenerated the pb2 modules, set them in the client-side conversion and read them defensively in a new internal_event_from_grpc helper on the server - gRPC metadata is transported as map<string,string>, which stringified wait markers. The wait-graph now parses metadata leniently, the string False no longer counts as a blocking wait and numeric strings keep their value as the edge deadline - edges forming a detected cycle are removed before the notification is delivered. The notified waiters return immediately, so keeping their edges alive until the original timeout made later unrelated waits between the same agents look like fresh deadlocks - send_and_wait now applies its internal blocking_wait / wait_timeout metadata after caller-supplied metadata, so a caller cannot disable detection or forge the deadline Adds an Event to gRPC to Event roundtrip test for the correlation fields, plus wait-graph tests for stringified metadata and for post-notification edge cleanup.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
In the mesh (logical full-mesh over a central network server), two agents waiting on each other's results deadlock. Three distinct failure modes existed:
send_and_waitinsidereact()stalls the agent's serial event loop; if the peer blocks the same way, both hang until timeout. Worse, waiters matched replies bysource_idonly, so a counter-request could be silently mistaken for a reply, and a fast reply could be lost entirely because the waiter was registered after sending (lost-wakeup race).Fix
Sync-wait governance (P0)
AgentClient.expect_event/EventWaiter), closing the lost-wakeup race.Event.response_tofield; the messaging mod propagatesresponse_to/requires_responseacross the direct-message relay and exposesmessage_idso receivers can reply with correlation (WorkerAgent.reply_direct). Counter-requests are no longer mistaken for replies; uncorrelated legacy replies still work via a fallback.react()execution is tracked via a ContextVar bound to its owning asyncio task; blocking wait primitives called insidereact()emit aDeprecationWarning(OPENAGENTS_STRICT_NO_BLOCKING_WAIT=1upgrades to an error). Background tasks spawned insidereact()and agent-to-mod RPCs are exempt.AgentClient.wait_mod_messagedid not exist — channelwait_for_reply/wait_for_post/wait_for_reaction/post_and_waitandWorkspace.channels(refresh=True)always raisedAttributeErrorinternally and silently returnedNone; their match conditions also targeted event shapes the messaging mod never emits. Both fixed;post_and_waitnow correlates replies to the posted message id.Delegation lineage (P1)
parent_task_id; the server derivesroot_task_id/delegation_chain/delegation_depthfrom the task store (clients cannot forge chains). Rejects self-delegation (agent-id normalization preventsagent:alias bypass), cycles (delegation_cycle_detected), depth overmax_delegation_depth, and fan-out overmax_active_delegations_per_agent— validation and task creation run under one lock so concurrent delegations cannot race past the fuse. Covers bothtask.delegateand capability-routed paths; assignment notifications carry lineage so assignees can extend the chain.Wait-graph deadlock detection (P1)
WaitGraphMonitor: blocking primitives mark requests withblocking_wait/wait_timeoutmetadata (plainrequires_responsecontinuation requests never create edges), edges clear on matchingresponse_toor expire at the waiter's own deadline. When an edge closes a cycle, every agent on it receivesagent.wait.deadlock_detected(bypassing subscription filters) naming the exact request ids;send_and_waitfails fast with the cycle path instead of timing out blind. Cycle edges are removed on notification so stale edges cannot re-trigger. Endpoints are canonicalized; self-waits are reported as single-node cycles. This catches what lineage checks cannot see: mutualsend_and_wait, independent tasks pointing at each other, and mixed-mechanism cycles.requires_response/response_tofields (backward-compatible tags 11/12, pb2 regenerated) so correlation survives the transport; stringified gRPC metadata is parsed leniently.Hygiene (P2)
task.completeresponses no longer reportcompleted_atasNone(stale snapshot); locally timed-out external tasks stop their background polling; gRPCSendEventnow has a 30s deadline; docs teach the continuation pattern and the no-blocking-wait-in-react rule.Testing
64 new tests across
tests/workspace/test_sync_wait_governance.py,tests/mods/test_task_delegation.py(TestDelegationLineage),tests/network/test_wait_graph.py,tests/grpc/test_grpc_send_event_deadline.py,tests/grpc/test_grpc_event_roundtrip.py— covering mutual-wait warning/strict modes, lost-wakeup, reply misclassification, concurrent waiters, deadlock fast-fail, self/parent-child/three-hop delegation cycles, depth and fan-out fuses (including concurrency), gRPC field roundtrip, and all hygiene regressions. All affected suites pass (96 passed); the remaining errors intests/grpc//integration suites are a pre-existing missingexamples/workspace_test.yamlfixture path that fails identically ondevelop.Addressed two rounds of external review (6 + 2 blockers, all resolved in the last two commits).