chore(wip): checkpoint P8/C avant chantier Codex inter-agents
Sauvegarde de l'arbre de travail en cours (persistance P8, conversations C-series, write-portal frontend, médiation d'entrée) avant d'attaquer le support de la délégation inter-agents pour les profils Codex. Le round-trip inter-agent question/réponse est couvert sans tokens par les tests loopback existants (state::mcp_e2e_loopback_tests). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@ -49,14 +49,18 @@ use std::time::Duration;
|
||||
use interprocess::local_socket::tokio::Stream as LocalSocketStream;
|
||||
use interprocess::local_socket::traits::tokio::Stream as _;
|
||||
use interprocess::local_socket::{GenericFilePath, ToFsName as _};
|
||||
use tokio::io::{
|
||||
AsyncBufReadExt, AsyncRead, AsyncWrite, AsyncWriteExt, BufReader,
|
||||
};
|
||||
use tokio::io::{AsyncBufReadExt, AsyncRead, AsyncWrite, AsyncWriteExt, BufReader};
|
||||
|
||||
/// How long the bridge waits to connect to the project loopback before giving up.
|
||||
/// Bounded so an absent/unreachable endpoint fails fast instead of hanging.
|
||||
const CONNECT_TIMEOUT: Duration = Duration::from_secs(5);
|
||||
|
||||
/// After the CLI closes stdin, how long to keep draining the loopback so an
|
||||
/// in-flight response still reaches the CLI. Bounded so the bridge never blocks
|
||||
/// waiting on the server: when stdin closes the CLI is gone, and the OS closes the
|
||||
/// loopback fd at process exit anyway.
|
||||
const DRAIN_GRACE: Duration = Duration::from_secs(1);
|
||||
|
||||
/// Parsed `mcp-server` invocation arguments.
|
||||
///
|
||||
/// `--endpoint` is **required** (without it the bridge has nowhere to relay).
|
||||
@ -222,18 +226,26 @@ async fn connect_loopback(endpoint: &str) -> Result<LocalSocketStream, String> {
|
||||
///
|
||||
/// Sequence:
|
||||
/// 1. Write the **handshake line** (`project`+`requester`) to the loopback.
|
||||
/// 2. Loop: read one **line** from the CLI (`cli_in`) → write it to the loopback;
|
||||
/// read one **line** from the loopback → write it to the CLI (`cli_out`).
|
||||
/// 3. **CLI stdin EOF** ⇒ stop, return `Ok(())` (clean code 0). Dropping the
|
||||
/// loopback writer closes that direction of the connection.
|
||||
/// 2. Run **two independent directional pumps concurrently** (full duplex):
|
||||
/// - `CLI → loopback`: every line from `cli_in` is forwarded to the loopback;
|
||||
/// - `loopback → CLI`: every line from the loopback is forwarded to `cli_out`.
|
||||
/// 3. **CLI stdin EOF** ⇒ half-close the loopback write side (so the server sees
|
||||
/// EOF), **drain** any responses still in flight, then return `Ok(())`.
|
||||
///
|
||||
/// The loop is request/response paced (one CLI line in, one loopback line back),
|
||||
/// matching the JSON Lines `StdioTransport` the server speaks. A loopback that
|
||||
/// closes mid-exchange surfaces as an error (non-zero exit).
|
||||
/// ## Why full duplex (and not lockstep)
|
||||
///
|
||||
/// JSON-RPC over MCP is **not** one-response-per-request. **Notifications** (e.g.
|
||||
/// `notifications/initialized` sent right after `initialize`) carry no `id` and get
|
||||
/// **no response**, and the server may push messages unsolicited. A lockstep pump
|
||||
/// that reads one CLI line then *blocks* for exactly one loopback line **deadlocks**
|
||||
/// on the first notification: it waits forever for a reply that never comes, and
|
||||
/// never reads the client's next request (e.g. `tools/list`) — so the CLI never
|
||||
/// receives its tool list. Two decoupled pumps let notifications and asynchronous
|
||||
/// server messages flow freely in both directions.
|
||||
async fn relay<CIn, COut, LIn, LOut>(
|
||||
args: &BridgeArgs,
|
||||
cli_in: CIn,
|
||||
mut cli_out: COut,
|
||||
cli_out: COut,
|
||||
lp_read: LIn,
|
||||
mut lp_write: LOut,
|
||||
) -> Result<(), String>
|
||||
@ -253,48 +265,66 @@ where
|
||||
.await
|
||||
.map_err(|e| format!("handshake flush failed: {e}"))?;
|
||||
|
||||
let mut cli_reader = BufReader::new(cli_in);
|
||||
let mut lp_reader = BufReader::new(lp_read);
|
||||
let mut cli_line = String::new();
|
||||
let mut lp_line = String::new();
|
||||
// 2. Two decoupled pumps. Each owns its streams so neither blocks the other.
|
||||
let cli_to_lp = pump_lines(BufReader::new(cli_in), lp_write, "stdin", "loopback");
|
||||
let lp_to_cli = pump_lines(BufReader::new(lp_read), cli_out, "loopback", "stdout");
|
||||
tokio::pin!(cli_to_lp);
|
||||
tokio::pin!(lp_to_cli);
|
||||
|
||||
tokio::select! {
|
||||
// CLI closed stdin (or errored): half-close the loopback writer to nudge
|
||||
// the server, then drain briefly so an in-flight response still reaches the
|
||||
// CLI — but never block on the server (bounded by DRAIN_GRACE).
|
||||
r = &mut cli_to_lp => {
|
||||
let mut lp_write = r?;
|
||||
let _ = lp_write.shutdown().await;
|
||||
let _ = tokio::time::timeout(DRAIN_GRACE, &mut lp_to_cli).await;
|
||||
Ok(())
|
||||
}
|
||||
// Loopback closed first (server hangup): nothing left to relay. The CLI's
|
||||
// stdin EOF is no longer needed to exit.
|
||||
r = &mut lp_to_cli => r.map(|_| ()),
|
||||
}
|
||||
}
|
||||
|
||||
/// Forwards every newline-delimited line from `reader` to `writer` until `reader`
|
||||
/// hits EOF, flushing after each line so a peer blocked on a read sees it promptly.
|
||||
///
|
||||
/// On clean EOF it returns the (now-drained) `writer` so the caller can half-close
|
||||
/// it. `src`/`dst` name the two ends for error messages only.
|
||||
async fn pump_lines<R, W>(
|
||||
mut reader: BufReader<R>,
|
||||
mut writer: W,
|
||||
src: &str,
|
||||
dst: &str,
|
||||
) -> Result<W, String>
|
||||
where
|
||||
R: AsyncRead + Unpin,
|
||||
W: AsyncWrite + Unpin,
|
||||
{
|
||||
let mut line = String::new();
|
||||
loop {
|
||||
// 2a. CLI → loopback (one line). EOF ⇒ clean shutdown.
|
||||
cli_line.clear();
|
||||
let n = cli_reader
|
||||
.read_line(&mut cli_line)
|
||||
line.clear();
|
||||
let n = reader
|
||||
.read_line(&mut line)
|
||||
.await
|
||||
.map_err(|e| format!("stdin read failed: {e}"))?;
|
||||
.map_err(|e| format!("{src} read failed: {e}"))?;
|
||||
if n == 0 {
|
||||
// CLI closed stdin: nothing more to forward. Clean exit.
|
||||
return Ok(());
|
||||
// Source closed: drain and hand the writer back for half-close.
|
||||
writer
|
||||
.flush()
|
||||
.await
|
||||
.map_err(|e| format!("{dst} flush failed: {e}"))?;
|
||||
return Ok(writer);
|
||||
}
|
||||
lp_write
|
||||
.write_all(cli_line.as_bytes())
|
||||
writer
|
||||
.write_all(line.as_bytes())
|
||||
.await
|
||||
.map_err(|e| format!("loopback write failed: {e}"))?;
|
||||
lp_write
|
||||
.map_err(|e| format!("{dst} write failed: {e}"))?;
|
||||
writer
|
||||
.flush()
|
||||
.await
|
||||
.map_err(|e| format!("loopback flush failed: {e}"))?;
|
||||
|
||||
// 2b. loopback → CLI (one line). EOF here is a server hangup mid-exchange.
|
||||
lp_line.clear();
|
||||
let m = lp_reader
|
||||
.read_line(&mut lp_line)
|
||||
.await
|
||||
.map_err(|e| format!("loopback read failed: {e}"))?;
|
||||
if m == 0 {
|
||||
return Err("loopback closed before answering".to_string());
|
||||
}
|
||||
cli_out
|
||||
.write_all(lp_line.as_bytes())
|
||||
.await
|
||||
.map_err(|e| format!("stdout write failed: {e}"))?;
|
||||
cli_out
|
||||
.flush()
|
||||
.await
|
||||
.map_err(|e| format!("stdout flush failed: {e}"))?;
|
||||
.map_err(|e| format!("{dst} flush failed: {e}"))?;
|
||||
}
|
||||
}
|
||||
|
||||
@ -313,7 +343,12 @@ mod tests {
|
||||
#[test]
|
||||
fn parses_all_three_flags() {
|
||||
let a = BridgeArgs::parse([
|
||||
"--endpoint", "/tmp/x.sock", "--project", "p1", "--requester", "agent-7",
|
||||
"--endpoint",
|
||||
"/tmp/x.sock",
|
||||
"--project",
|
||||
"p1",
|
||||
"--requester",
|
||||
"agent-7",
|
||||
])
|
||||
.unwrap();
|
||||
assert_eq!(a.endpoint, "/tmp/x.sock");
|
||||
@ -355,8 +390,7 @@ mod tests {
|
||||
};
|
||||
let line = a.handshake_line();
|
||||
assert_eq!(*line.last().unwrap(), b'\n');
|
||||
let v: serde_json::Value =
|
||||
serde_json::from_slice(&line[..line.len() - 1]).unwrap();
|
||||
let v: serde_json::Value = serde_json::from_slice(&line[..line.len() - 1]).unwrap();
|
||||
assert_eq!(v["project"], "proj");
|
||||
assert_eq!(v["requester"], "rq");
|
||||
}
|
||||
@ -393,24 +427,19 @@ mod tests {
|
||||
reader.read_line(&mut request).await.unwrap();
|
||||
|
||||
let mut w = srv_write;
|
||||
w.write_all(b"{\"result\":\"ok\",\"id\":1}\n").await.unwrap();
|
||||
w.write_all(b"{\"result\":\"ok\",\"id\":1}\n")
|
||||
.await
|
||||
.unwrap();
|
||||
w.flush().await.unwrap();
|
||||
(handshake, request)
|
||||
});
|
||||
|
||||
relay(
|
||||
&args,
|
||||
&cli_in[..],
|
||||
&mut cli_out,
|
||||
lp_read,
|
||||
lp_write,
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
relay(&args, &cli_in[..], &mut cli_out, lp_read, lp_write)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let (handshake, request) = server.await.unwrap();
|
||||
let hs: serde_json::Value =
|
||||
serde_json::from_str(handshake.trim_end()).unwrap();
|
||||
let hs: serde_json::Value = serde_json::from_str(handshake.trim_end()).unwrap();
|
||||
assert_eq!(hs["project"], "proj-1");
|
||||
assert_eq!(hs["requester"], "agent-9");
|
||||
assert_eq!(request.trim_end(), "{\"method\":\"tools/call\",\"id\":1}");
|
||||
@ -420,6 +449,69 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// Regression (the bug that hid the tool list for days): a **notification**
|
||||
/// (no `id`, no response) sent between two requests must NOT stall the relay.
|
||||
/// A lockstep pump deadlocks here — after forwarding the notification it blocks
|
||||
/// waiting for a reply that never comes, and never reads `tools/list`. The
|
||||
/// full-duplex relay forwards all three lines and delivers the one response.
|
||||
#[tokio::test]
|
||||
async fn relay_does_not_block_on_notification_without_response() {
|
||||
// initialize result, then an unanswered notification, then tools/list.
|
||||
let cli_in = b"{\"method\":\"initialize\",\"id\":1}\n\
|
||||
{\"method\":\"notifications/initialized\"}\n\
|
||||
{\"method\":\"tools/list\",\"id\":2}\n"
|
||||
.to_vec();
|
||||
let mut cli_out: Vec<u8> = Vec::new();
|
||||
|
||||
let (lp_read, srv_write) = tokio::io::duplex(4096); // server → bridge
|
||||
let (srv_read, lp_write) = tokio::io::duplex(4096); // bridge → server
|
||||
|
||||
let args = BridgeArgs {
|
||||
endpoint: "unused".into(),
|
||||
project: "p".into(),
|
||||
requester: "agent".into(),
|
||||
};
|
||||
|
||||
// Server: handshake, then read all three forwarded lines, answering only
|
||||
// the two that carry an `id`. The notification gets no reply — exactly the
|
||||
// case that used to wedge the relay.
|
||||
let server = tokio::spawn(async move {
|
||||
let mut reader = BufReader::new(srv_read);
|
||||
let mut w = srv_write;
|
||||
for _ in 0..4 {
|
||||
let mut line = String::new();
|
||||
if reader.read_line(&mut line).await.unwrap() == 0 {
|
||||
break;
|
||||
}
|
||||
let v: serde_json::Value = match serde_json::from_str(line.trim_end()) {
|
||||
Ok(v) => v,
|
||||
Err(_) => continue, // handshake line
|
||||
};
|
||||
if let Some(id) = v.get("id") {
|
||||
w.write_all(format!("{{\"result\":\"ok\",\"id\":{id}}}\n").as_bytes())
|
||||
.await
|
||||
.unwrap();
|
||||
w.flush().await.unwrap();
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
tokio::time::timeout(
|
||||
Duration::from_secs(5),
|
||||
relay(&args, &cli_in[..], &mut cli_out, lp_read, lp_write),
|
||||
)
|
||||
.await
|
||||
.expect("relay must not deadlock on a notification")
|
||||
.expect("relay ok");
|
||||
|
||||
server.await.unwrap();
|
||||
|
||||
let out = String::from_utf8(cli_out).unwrap();
|
||||
// Both request responses arrived; the notification produced none.
|
||||
assert!(out.contains("\"id\":1"), "missing initialize response: {out}");
|
||||
assert!(out.contains("\"id\":2"), "missing tools/list response: {out}");
|
||||
}
|
||||
|
||||
/// stdin EOF with no request ⇒ relay returns `Ok` (code 0), having still sent
|
||||
/// the handshake.
|
||||
#[tokio::test]
|
||||
@ -485,12 +577,9 @@ mod tests {
|
||||
#[tokio::test]
|
||||
async fn connect_to_absent_endpoint_errors_fast() {
|
||||
let endpoint = temp_endpoint("absent");
|
||||
let res = tokio::time::timeout(
|
||||
Duration::from_secs(10),
|
||||
connect_loopback(&endpoint),
|
||||
)
|
||||
.await
|
||||
.expect("connect_loopback must not hang");
|
||||
let res = tokio::time::timeout(Duration::from_secs(10), connect_loopback(&endpoint))
|
||||
.await
|
||||
.expect("connect_loopback must not hang");
|
||||
assert!(res.is_err(), "absent endpoint must yield an error");
|
||||
}
|
||||
|
||||
@ -545,13 +634,10 @@ mod tests {
|
||||
|
||||
// Drive the whole bridge_over_loopback path (real connect + split + relay),
|
||||
// but feed our own stdio streams via `relay` to avoid touching process std.
|
||||
let conn = tokio::time::timeout(
|
||||
Duration::from_secs(10),
|
||||
connect_loopback(&endpoint),
|
||||
)
|
||||
.await
|
||||
.expect("no hang")
|
||||
.expect("connect to live endpoint");
|
||||
let conn = tokio::time::timeout(Duration::from_secs(10), connect_loopback(&endpoint))
|
||||
.await
|
||||
.expect("no hang")
|
||||
.expect("connect to live endpoint");
|
||||
let (lp_read, lp_write) = tokio::io::split(conn);
|
||||
|
||||
tokio::time::timeout(
|
||||
@ -565,8 +651,7 @@ mod tests {
|
||||
server.await.unwrap();
|
||||
|
||||
let (handshake, request) = captured.lock().await.clone();
|
||||
let hs: serde_json::Value =
|
||||
serde_json::from_str(handshake.trim_end()).unwrap();
|
||||
let hs: serde_json::Value = serde_json::from_str(handshake.trim_end()).unwrap();
|
||||
assert_eq!(hs["project"], "proj-e2e");
|
||||
assert_eq!(hs["requester"], "agent-e2e");
|
||||
assert_eq!(request.trim_end(), "{\"method\":\"ping\",\"id\":42}");
|
||||
|
||||
Reference in New Issue
Block a user