From 5a0861d639cf4895a9b3ca9e5b886fe1c2f8bce7 Mon Sep 17 00:00:00 2001 From: rob thijssen Date: Thu, 28 May 2026 12:16:21 +0300 Subject: [PATCH] fix(helexa-acp): forward Dispatch::Response to its awaiting router MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The catch-all on_receive_dispatch handler was applying respond_with_error to *every* Dispatch variant, including Response. For Response variants, that call routes the error to the ResponseRouter for the *outgoing* request — silently overwriting the real reply from Zed with "Internal error: not implemented yet". Every ACP roundtrip we issue (fs/read_text_file, fs/write_text_file, session/request_permission, terminal/*) was therefore returning an error to the tool runner regardless of what Zed actually responded. The model saw uniformly-failing tools, gave up, and confabulated plausible explanations. Fix: pattern-match the Dispatch. Response → forward to its router via respond_with_result. Request / Notification → keep the "not implemented yet" error response as before. Found via debug logs showing WARN helexa_acp::agent: unhandled ACP message method="fs/read_text_file" right before every tool failure. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/helexa-acp/src/agent.rs | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/crates/helexa-acp/src/agent.rs b/crates/helexa-acp/src/agent.rs index 189a697..d736a32 100644 --- a/crates/helexa-acp/src/agent.rs +++ b/crates/helexa-acp/src/agent.rs @@ -158,11 +158,31 @@ impl Agent { ) .on_receive_dispatch( async move |message: Dispatch, cx: ConnectionTo| { - tracing::warn!(method = ?message.method(), "unhandled ACP message"); - message.respond_with_error( - agent_client_protocol::util::internal_error("not implemented yet"), - cx, - ) + // `Dispatch` has three variants. For Request and + // Notification we want the "not implemented yet" + // error response. For *Response* we MUST forward + // the result to its awaiting `ResponseRouter` — + // otherwise our own outbound ACP calls + // (`fs/read_text_file`, `session/request_permission`, + // `terminal/*`, …) get their replies silently + // overwritten with whatever error we'd send a + // peer for an unknown method. That's how Stage 3 + // tool dispatches were appearing as + // "Internal error: not implemented yet" results + // to the model. + match message { + Dispatch::Response(result, router) => router.respond_with_result(result), + other => { + tracing::warn!( + method = ?other.method(), + "unhandled ACP message" + ); + other.respond_with_error( + agent_client_protocol::util::internal_error("not implemented yet"), + cx, + ) + } + } }, agent_client_protocol::on_receive_dispatch!(), )