Skip to content

Commit 63133e6

Browse files
committed
fix(acp): drain failed initialize negotiation state
1 parent d663209 commit 63133e6

2 files changed

Lines changed: 40 additions & 26 deletions

File tree

src/agent-client-protocol/src/jsonrpc/outgoing_actor.rs

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -64,29 +64,29 @@ pub(super) async fn outgoing_protocol_actor(
6464
OutgoingMessage::Response {
6565
id,
6666
method,
67-
response: Ok(value),
68-
} => {
69-
let value = protocol_compat.outgoing_response(&method, Ok(value))?;
70-
tracing::debug!(?id, "Sending success response");
71-
jsonrpcmsg::Message::Response(jsonrpcmsg::Response::success_v2(value, Some(id)))
72-
}
73-
OutgoingMessage::Response {
74-
id,
75-
method,
76-
response: Err(error),
77-
} => {
78-
tracing::warn!(?id, %method, ?error, "Sending error response");
79-
// Convert crate::Error to jsonrpcmsg::Error
80-
let jsonrpc_error = jsonrpcmsg::Error {
81-
code: error.code.into(),
82-
message: error.message,
83-
data: error.data,
84-
};
85-
jsonrpcmsg::Message::Response(jsonrpcmsg::Response::error_v2(
86-
jsonrpc_error,
87-
Some(id),
88-
))
89-
}
67+
response,
68+
} => match protocol_compat.outgoing_response(&method, response) {
69+
Ok(value) => {
70+
tracing::debug!(?id, "Sending success response");
71+
jsonrpcmsg::Message::Response(jsonrpcmsg::Response::success_v2(
72+
value,
73+
Some(id),
74+
))
75+
}
76+
Err(error) => {
77+
tracing::warn!(?id, %method, ?error, "Sending error response");
78+
// Convert crate::Error to jsonrpcmsg::Error
79+
let jsonrpc_error = jsonrpcmsg::Error {
80+
code: error.code.into(),
81+
message: error.message,
82+
data: error.data,
83+
};
84+
jsonrpcmsg::Message::Response(jsonrpcmsg::Response::error_v2(
85+
jsonrpc_error,
86+
Some(id),
87+
))
88+
}
89+
},
9090
OutgoingMessage::Error { error } => {
9191
// Convert crate::Error to jsonrpcmsg::Error
9292
let jsonrpc_error = jsonrpcmsg::Error {

src/agent-client-protocol/src/jsonrpc/protocol_compat.rs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,11 @@ mod imp {
134134
(Self::Disabled, other) => other,
135135
(this, Self::Disabled) => this,
136136
(Self::Acp(this), Self::Acp(other)) => {
137+
debug_assert_eq!(
138+
this.api, other.api,
139+
"cannot merge ACP builders with different API protocol versions; \
140+
handler chains share a single API surface",
141+
);
137142
if this.latest_supported >= other.latest_supported {
138143
Self::Acp(this)
139144
} else {
@@ -250,13 +255,22 @@ mod imp {
250255
method: &str,
251256
result: Result<serde_json::Value, crate::Error>,
252257
) -> Result<serde_json::Value, crate::Error> {
253-
let mut value = result?;
254258
let Some(mode) = self.mode else {
255-
return Ok(value);
259+
return result;
260+
};
261+
262+
// Always drain any pending initialize state so a failed initialize
263+
// doesn't leak negotiation state to a subsequent request.
264+
let pending_initialize = if method == "initialize" {
265+
self.take_pending_initialize()
266+
} else {
267+
None
256268
};
257269

270+
let mut value = result?;
271+
258272
let wire_version = if method == "initialize" {
259-
let negotiated = self.take_pending_initialize().unwrap_or_else(|| {
273+
let negotiated = pending_initialize.unwrap_or_else(|| {
260274
protocol_version_from_value(&value)
261275
.and_then(ProtocolVersionKind::from_protocol_version)
262276
.unwrap_or(mode.latest_supported)

0 commit comments

Comments
 (0)