Skip to content

Commit bbed0a3

Browse files
authored
Merge pull request #18 from nikomatsakis/main
refactor conductor API for "lazy initialization", fix race condition
2 parents e1720e4 + b6b4a41 commit bbed0a3

File tree

7 files changed

+557
-203
lines changed

7 files changed

+557
-203
lines changed

md/architecture.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ The conductor uses `_proxy/successor/request` bidirectionally with different mea
4646
- **Proxy → Conductor**: Proxy sends `_proxy/successor/request` to send a message TO its successor (downstream)
4747
- **Conductor → Proxy**: Conductor sends `_proxy/successor/request` to deliver a message FROM the successor (upstream)
4848

49+
**Important**: The conductor maintains message ordering by routing all forwarding decisions through a central event loop, preventing responses from overtaking notifications even though they use different transport paths.
50+
4951
**Downstream flow (Proxy 1 sending to Proxy 2):**
5052
1. Conductor sends normal ACP request to Proxy 1
5153
2. Proxy 1 sends `_proxy/successor/request` to conductor (meaning "send this TO my successor")
@@ -68,8 +70,9 @@ The **conductor** is the orchestrator that manages the proxy chain. From the edi
6870

6971
**Responsibilities:**
7072
1. **Process Management**: Spawns and manages component processes
71-
2. **Message Routing**: Routes messages through the proxy chain
73+
2. **Message Routing**: Routes messages through the proxy chain, preserving send order
7274
3. **Capability Adaptation**: Bridges between different component capabilities
75+
4. **Message Ordering**: Ensures all messages maintain send order when forwarded through proxy chains
7376

7477
**Usage:**
7578
```bash

md/conductor.md

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ conductor mcp 54321
5858

5959
The conductor routes ALL messages between components. No component talks directly to another.
6060

61+
**Message ordering**: The conductor preserves message send order by routing all forwarding decisions through a central event loop, preventing responses from overtaking notifications.
62+
6163
**Message flow types:**
6264

6365
1. **Editor → First Component**: Conductor forwards normal ACP messages
@@ -200,6 +202,31 @@ The conductor uses an actor-based architecture with message passing via channels
200202
- **Message router**: Central actor that receives `ConductorMessage` enums and routes appropriately
201203
- **MCP bridge actors**: Manage MCP-over-ACP connections
202204

205+
### Message Ordering Invariant
206+
207+
**Critical invariant**: All messages (requests, responses, notifications) between any two endpoints must maintain their send order.
208+
209+
The conductor ensures this invariant by routing **all** message forwarding through its central message queue (`ConductorMessage` channel). This prevents faster message types (responses) from overtaking slower ones (notifications).
210+
211+
#### Why This Matters
212+
213+
Without ordering preservation, a race condition can occur:
214+
1. Agent sends `session/update` notification
215+
2. Agent responds to `session/prompt` request
216+
3. Response takes a fast path (reply_actor with oneshot channels)
217+
4. Notification takes slower path (handler pipeline)
218+
5. **Response arrives before notification** → client loses notification data
219+
220+
#### Implementation
221+
222+
The conductor uses extension traits to route all forwarding through the central queue:
223+
224+
- `JrConnectionCxExt::send_proxied_message_via` - Routes both requests and notifications
225+
- `JrRequestCxExt::respond_via` - Routes responses through the queue
226+
- `JrResponseExt::forward_response_via` - Ensures response forwarding maintains order
227+
228+
All message forwarding in both directions (client-to-agent and agent-to-client) flows through the conductor's central event loop, which processes `ConductorMessage` enums sequentially. This serialization ensures messages arrive in the same order they were sent.
229+
203230
### Message Routing Implementation
204231

205232
The conductor uses a recursive spawning pattern:

md/transport-architecture.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,14 @@ Incoming Protocol Actor
154154
Handler or Reply Actor
155155
```
156156

157+
### Message Ordering in the Conductor
158+
159+
When the conductor forwards messages between components, it must preserve send order to prevent race conditions. The conductor achieves this by routing all message forwarding through a central message queue.
160+
161+
**Key insight**: While the transport actors operate independently, the **conductor's routing logic** serializes all forwarding decisions through a central event loop. This ensures that even though responses use a "fast path" (reply_actor with oneshot channels) at the transport level, the decision to forward them is serialized with notification forwarding at the protocol level.
162+
163+
Without this serialization, responses could overtake notifications when both are forwarded through proxy chains, causing the client to receive messages out of order. See [Conductor Implementation](./conductor.md#message-ordering-invariant) for details.
164+
157165
## Transport Trait
158166

159167
The `IntoJrConnectionTransport` trait defines how to bridge internal channels with I/O:

0 commit comments

Comments
 (0)