From 4aa78ff83f30f4b760daecbcabb4e40d8efa4264 Mon Sep 17 00:00:00 2001 From: Tom Dupuis <60640908+tomdps@users.noreply.github.com> Date: Fri, 24 Jul 2026 22:49:41 +0200 Subject: [PATCH] feat: implement #749 - feat(cluster-protocol): add capability-gated read-only agent attach --- .../src/agent_attach.rs | 30 ++ crates/openengine-cluster-client/src/lib.rs | 22 +- .../src/ndjson_agent_attach.rs | 23 ++ .../src/ndjson_logs.rs | 164 +-------- .../src/ndjson_subscription.rs | 175 ++++++++++ .../src/ndjson_watch.rs | 2 +- .../tests/agent_attach.rs | 154 +++++++++ .../tests/cancel_leak_support/mod.rs | 105 ++++++ .../openengine-cluster-client/tests/logs.rs | 58 +--- .../src/agent_attach.rs | 161 +++++++++ crates/openengine-cluster-protocol/src/lib.rs | 17 + .../openengine-cluster-protocol/src/logs.rs | 115 +------ .../openengine-cluster-protocol/src/wire.rs | 125 +++++++ .../tests/agent_attach.rs | 254 ++++++++++++++ .../tests/capabilities.rs | 15 +- .../src/agent_attach.rs | 99 ++++++ .../src/agent_attach/fixtures.rs | 167 +++++++++ .../src/agent_attach/ports.rs | 37 ++ crates/openengine-cluster-server/src/lib.rs | 24 +- crates/openengine-cluster-server/src/logs.rs | 122 +------ crates/openengine-cluster-server/src/stdio.rs | 185 +++++----- .../src/stdio/agent_attach.rs | 101 ++++++ .../src/stdio/logs.rs | 80 ++--- .../src/stdio/subscription.rs | 190 +++++++++++ .../src/stdio/tests.rs | 55 +++ .../src/subscription_stream.rs | 131 ++++++++ .../tests/agent_attach.rs | 317 ++++++++++++++++++ .../tests/capability_default_support/mod.rs | 20 ++ .../tests/initialize_capabilities.rs | 1 + .../openengine-cluster-server/tests/logs.rs | 110 +++--- .../tests/ndjson_test_support/mod.rs | 4 +- .../tests/oversized_event_wire_support/mod.rs | 48 +++ .../tests/oversized_id_backend_support/mod.rs | 62 ++++ .../src/agent_attach.rs | 7 + .../src/agent_attach_artifacts.rs | 124 +++++++ .../src/artifacts.rs | 20 +- .../src/artifacts/openrpc.rs | 35 +- .../src/capability_vectors.rs | 10 + crates/openengine-cluster-testkit/src/lib.rs | 2 + .../src/logs_artifacts.rs | 2 +- .../tests/agent_attach.rs | 89 +++++ .../tests/artifacts.rs | 13 +- .../tests/capability_vectors.rs | 1 + .../openengine-cluster-testkit/tests/logs.rs | 12 +- .../tests/protocol_v1.rs | 2 +- .../tests/schema_support/mod.rs | 13 + .../agent_attach/agent-attach-closed.json | 10 + .../agent_attach/agent-attach-event.json | 16 + .../agent_attach/agent-attach-params.json | 3 + .../v1/goldens/admission-lifecycle.ndjson | 2 +- .../v1/goldens/agent-attach-session.json | 28 ++ .../v1/goldens/initialize.ndjson | 2 +- protocol/openengine-cluster/v1/openrpc.json | 24 +- protocol/openengine-cluster/v1/schema.json | 227 ++++++++++++- zeroshot-rust/tests/backend_boundary.rs | 4 +- 55 files changed, 3133 insertions(+), 686 deletions(-) create mode 100644 crates/openengine-cluster-client/src/agent_attach.rs create mode 100644 crates/openengine-cluster-client/src/ndjson_agent_attach.rs create mode 100644 crates/openengine-cluster-client/src/ndjson_subscription.rs create mode 100644 crates/openengine-cluster-client/tests/agent_attach.rs create mode 100644 crates/openengine-cluster-client/tests/cancel_leak_support/mod.rs create mode 100644 crates/openengine-cluster-protocol/src/agent_attach.rs create mode 100644 crates/openengine-cluster-protocol/tests/agent_attach.rs create mode 100644 crates/openengine-cluster-server/src/agent_attach.rs create mode 100644 crates/openengine-cluster-server/src/agent_attach/fixtures.rs create mode 100644 crates/openengine-cluster-server/src/agent_attach/ports.rs create mode 100644 crates/openengine-cluster-server/src/stdio/agent_attach.rs create mode 100644 crates/openengine-cluster-server/src/stdio/subscription.rs create mode 100644 crates/openengine-cluster-server/src/stdio/tests.rs create mode 100644 crates/openengine-cluster-server/src/subscription_stream.rs create mode 100644 crates/openengine-cluster-server/tests/agent_attach.rs create mode 100644 crates/openengine-cluster-server/tests/capability_default_support/mod.rs create mode 100644 crates/openengine-cluster-server/tests/oversized_event_wire_support/mod.rs create mode 100644 crates/openengine-cluster-server/tests/oversized_id_backend_support/mod.rs create mode 100644 crates/openengine-cluster-testkit/src/agent_attach.rs create mode 100644 crates/openengine-cluster-testkit/src/agent_attach_artifacts.rs create mode 100644 crates/openengine-cluster-testkit/tests/agent_attach.rs create mode 100644 crates/openengine-cluster-testkit/tests/schema_support/mod.rs create mode 100644 protocol/openengine-cluster/v1/fixtures/agent_attach/agent-attach-closed.json create mode 100644 protocol/openengine-cluster/v1/fixtures/agent_attach/agent-attach-event.json create mode 100644 protocol/openengine-cluster/v1/fixtures/agent_attach/agent-attach-params.json create mode 100644 protocol/openengine-cluster/v1/goldens/agent-attach-session.json diff --git a/crates/openengine-cluster-client/src/agent_attach.rs b/crates/openengine-cluster-client/src/agent_attach.rs new file mode 100644 index 00000000..9da81115 --- /dev/null +++ b/crates/openengine-cluster-client/src/agent_attach.rs @@ -0,0 +1,30 @@ +//! Typed in-process `agent/attach` subscription client. Wraps a [`Dispatcher`] directly since +//! NDJSON/WebSocket subscription framing is bound by a later issue. No dedup or reconnect logic +//! exists here -- `agent/attach` has no cursor to resume from. + +use openengine_cluster_protocol::{AgentAttachParams, AgentAttachResult}; +use openengine_cluster_server::agent_attach::{AgentAttachEventStream, AgentAttachHandle}; +use openengine_cluster_server::{BackendError, ClusterBackend, Dispatcher}; + +/// Typed in-process `agent/attach` client. Wraps a [`Dispatcher`] directly since NDJSON/WebSocket +/// subscription framing is bound by a later issue. +pub struct AgentAttachClient { + dispatcher: Dispatcher, +} + +impl AgentAttachClient +where + B: ClusterBackend, +{ + #[must_use] + pub const fn new(dispatcher: Dispatcher) -> Self { + Self { dispatcher } + } + + pub async fn agent_attach( + &self, + params: AgentAttachParams, + ) -> Result<(AgentAttachResult, AgentAttachEventStream, AgentAttachHandle), BackendError> { + self.dispatcher.agent_attach(params).await + } +} diff --git a/crates/openengine-cluster-client/src/lib.rs b/crates/openengine-cluster-client/src/lib.rs index 6ac07ab5..c4f9e565 100644 --- a/crates/openengine-cluster-client/src/lib.rs +++ b/crates/openengine-cluster-client/src/lib.rs @@ -1,14 +1,19 @@ //! Typed transport-neutral Cluster Protocol client. mod ndjson_pump; +mod ndjson_subscription; use ndjson_pump::forward_notification; +pub mod agent_attach; pub mod logs; +pub mod ndjson_agent_attach; pub mod ndjson_logs; pub mod ndjson_watch; pub mod watch; +pub use agent_attach::*; pub use logs::*; +pub use ndjson_agent_attach::*; pub use ndjson_logs::*; pub use ndjson_watch::*; pub use watch::*; @@ -187,19 +192,20 @@ where }) } - /// Sends a `watch` request and returns its response line plus the receiver registered for its - /// subscription's notifications. Errors if the response carried no `subscriptionId` (either a - /// backend error, or a malformed/unexpected response). + /// Sends a subscription-establishing request and returns its response line plus, only on + /// success, the receiver registered for its subscription's notifications. Establishment can + /// legitimately fail with a JSON-RPC error (for example `agent/attach` rejecting an unknown or + /// inactive `ExecutionRef`) -- that case carries no `subscriptionId` but is not a transport + /// fault, so it is left for the caller's response parser to surface as a typed + /// [`crate::ClientError::Rpc`] rather than being collapsed into a generic [`TransportError`] + /// here. pub(crate) async fn open_subscription( &self, request: String, id: RequestId, - ) -> Result<(String, PumpedSubscription), TransportError> { + ) -> Result<(String, Option), TransportError> { let response = self.send_request(request, id).await?; - let subscription = response.subscription.ok_or_else(|| { - TransportError::Protocol("watch response carried no subscriptionId".to_owned()) - })?; - Ok((response.line, subscription)) + Ok((response.line, response.subscription)) } pub(crate) fn next_watch_request_id(&self) -> RequestId { diff --git a/crates/openengine-cluster-client/src/ndjson_agent_attach.rs b/crates/openengine-cluster-client/src/ndjson_agent_attach.rs new file mode 100644 index 00000000..d893fa08 --- /dev/null +++ b/crates/openengine-cluster-client/src/ndjson_agent_attach.rs @@ -0,0 +1,23 @@ +//! NDJSON-bound `agent/attach` subscription client. Drives `agent/attach`/`event`/ +//! `subscription/cancel`/`subscription/closed` notifications over [`crate::NdjsonTransport`], +//! reusing the exact same generic subscription framing [`crate::ndjson_watch`]/ +//! [`crate::ndjson_logs`] use. There is no dedup or reconnect logic here, unlike +//! [`crate::NdjsonReconnectingEventStream`] -- `agent/attach` has no cursor to resume from. + +use crate::ndjson_subscription::impl_ndjson_event_subscription; + +impl_ndjson_event_subscription! { + client: NdjsonAgentAttachClient, + stream: NdjsonAgentAttachEventStream, + event_or_closed: AgentAttachEventOrClosed, + method_fn: agent_attach, + method_name: "agent/attach", + params: openengine_cluster_protocol::AgentAttachParams, + result: openengine_cluster_protocol::AgentAttachResult, + event: openengine_cluster_protocol::AgentAttachEvent, + event_notification: openengine_cluster_protocol::AgentAttachEventNotification, + event_field: event, + closed_notification: openengine_cluster_protocol::AgentAttachClosedNotification, + parse_response_fn: parse_agent_attach_response, + parse_notification_fn: parse_agent_attach_notification, +} diff --git a/crates/openengine-cluster-client/src/ndjson_logs.rs b/crates/openengine-cluster-client/src/ndjson_logs.rs index 7932d3c3..dae280d2 100644 --- a/crates/openengine-cluster-client/src/ndjson_logs.rs +++ b/crates/openengine-cluster-client/src/ndjson_logs.rs @@ -3,152 +3,20 @@ //! generic subscription framing [`crate::ndjson_watch`] uses. There is no dedup or reconnect logic //! here, unlike [`crate::NdjsonReconnectingEventStream`] -- `logs` has no cursor to resume from. -use std::sync::atomic::Ordering; - -use openengine_cluster_protocol::{ - JsonRpcErrorResponse, JsonRpcNotification, JsonRpcRequest, JsonRpcSuccess, - LogEventNotification, LogRecord, LogsClosedNotification, LogsParams, LogsResult, RequestId, - SubscriptionCloseReason, SubscriptionId, JSON_RPC_VERSION, -}; -use serde_json::Value; -use tokio::io::{AsyncRead, AsyncWrite}; -use tokio::sync::mpsc; - -use crate::PumpedSubscription; -use crate::{validate_response_identity, ClientError, NdjsonTransport}; - -/// One item observed by [`NdjsonLogsEventStream`]: a live log record, or a terminal close. -#[derive(Clone, Debug, PartialEq)] -pub enum LogEventOrClosed { - Event(LogRecord), - Closed { reason: SubscriptionCloseReason }, -} - -/// Typed NDJSON `logs` client. Request ids come from the shared [`NdjsonTransport`] rather than a -/// client-local counter, so independently constructed subscription clients on one connection -/// cannot replace each other's pending response waiters. -pub struct NdjsonLogsClient<'a, R, W> { - transport: &'a NdjsonTransport, -} - -impl<'a, R, W> NdjsonLogsClient<'a, R, W> -where - R: AsyncRead + Send + Unpin + 'static, - W: AsyncWrite + Send + Unpin + 'static, -{ - #[must_use] - pub const fn new(transport: &'a NdjsonTransport) -> Self { - Self { transport } - } - - pub async fn logs( - &self, - params: LogsParams, - ) -> Result<(LogsResult, NdjsonLogsEventStream<'a, R, W>), ClientError> { - let id = self.transport.next_watch_request_id(); - let request = serde_json::to_string(&JsonRpcRequest { - jsonrpc: JSON_RPC_VERSION.to_owned(), - id: id.clone(), - method: "logs".to_owned(), - params, - })?; - let (line, subscription) = self - .transport - .open_subscription(request, id.clone()) - .await?; - let result = parse_logs_response(&line, &id)?; - let PumpedSubscription { - receiver, - overflowed, - } = subscription; - let stream = NdjsonLogsEventStream { - transport: self.transport, - receiver, - overflowed, - subscription_id: result.subscription_id.clone(), - }; - Ok((result, stream)) - } -} - -fn parse_logs_response(line: &str, expected_id: &RequestId) -> Result { - let value: Value = serde_json::from_str(line) - .map_err(|error| ClientError::InvalidResponse(error.to_string()))?; - if value.get("error").is_some() { - let response: JsonRpcErrorResponse = serde_json::from_value(value) - .map_err(|error| ClientError::InvalidResponse(error.to_string()))?; - validate_response_identity(&response.jsonrpc, response.id.as_ref(), expected_id)?; - return Err(ClientError::Rpc(response.error)); - } - let response: JsonRpcSuccess = serde_json::from_value(value) - .map_err(|error| ClientError::InvalidResponse(error.to_string()))?; - validate_response_identity(&response.jsonrpc, Some(&response.id), expected_id)?; - Ok(response.result) -} - -/// Sourced from wire notifications forwarded by [`NdjsonTransport`]'s pump. Unlike -/// [`crate::NdjsonReconnectingEventStream`], there is no dedup set and no reconnect: `logs` gives -/// no cursor to resume from, so a `SLOW_CONSUMER` close simply ends the subscription. -pub struct NdjsonLogsEventStream<'a, R, W> { - transport: &'a NdjsonTransport, - receiver: mpsc::Receiver, - overflowed: std::sync::Arc, - subscription_id: SubscriptionId, -} - -impl<'a, R, W> NdjsonLogsEventStream<'a, R, W> -where - R: AsyncRead + Send + Unpin + 'static, - W: AsyncWrite + Send + Unpin + 'static, -{ - /// Returns the next live log record, or a terminal close. Returns `None` once the - /// subscription's channel ends (cancelled locally, or the transport's connection ended). - /// Returns `Some(Err(_))` if a schema-malformed or unexpected-method notification is - /// forwarded for this subscription -- the wire pump routes by subscription id only, so - /// peer-controlled payload shape must never panic here. - pub async fn next(&mut self) -> Option> { - let line = match self.receiver.recv().await { - Some(line) => line, - None if self.overflowed.swap(false, Ordering::AcqRel) => { - return Some(Ok(LogEventOrClosed::Closed { - reason: SubscriptionCloseReason::SlowConsumer, - })); - } - None => return None, - }; - Some(parse_log_notification(&line)) - } - - /// Sends `subscription/cancel` for this subscription. Idempotent from the caller's - /// perspective: the server drops an unknown subscription id silently. - pub async fn cancel(&self) -> Result<(), ClientError> { - self.transport - .cancel_subscription(self.subscription_id.clone()) - .await?; - Ok(()) - } -} - -fn parse_log_notification(line: &str) -> Result { - let value: Value = serde_json::from_str(line) - .map_err(|error| ClientError::InvalidResponse(error.to_string()))?; - match value.get("method").and_then(Value::as_str) { - Some("event") => { - let notification: JsonRpcNotification = - serde_json::from_value(value) - .map_err(|error| ClientError::InvalidResponse(error.to_string()))?; - Ok(LogEventOrClosed::Event(notification.params.record)) - } - Some("subscription/closed") => { - let notification: JsonRpcNotification = - serde_json::from_value(value) - .map_err(|error| ClientError::InvalidResponse(error.to_string()))?; - Ok(LogEventOrClosed::Closed { - reason: notification.params.reason, - }) - } - other => Err(ClientError::InvalidResponse(format!( - "unexpected subscription notification method {other:?}" - ))), - } +use crate::ndjson_subscription::impl_ndjson_event_subscription; + +impl_ndjson_event_subscription! { + client: NdjsonLogsClient, + stream: NdjsonLogsEventStream, + event_or_closed: LogEventOrClosed, + method_fn: logs, + method_name: "logs", + params: openengine_cluster_protocol::LogsParams, + result: openengine_cluster_protocol::LogsResult, + event: openengine_cluster_protocol::LogRecord, + event_notification: openengine_cluster_protocol::LogEventNotification, + event_field: record, + closed_notification: openengine_cluster_protocol::LogsClosedNotification, + parse_response_fn: parse_logs_response, + parse_notification_fn: parse_log_notification, } diff --git a/crates/openengine-cluster-client/src/ndjson_subscription.rs b/crates/openengine-cluster-client/src/ndjson_subscription.rs new file mode 100644 index 00000000..93f205eb --- /dev/null +++ b/crates/openengine-cluster-client/src/ndjson_subscription.rs @@ -0,0 +1,175 @@ +//! Shared NDJSON "one unary response, then live `event`/`subscription/closed` notifications with +//! no dedup or reconnect" client machinery for future-only subscription capabilities (`logs`, +//! `agent_attach`). Generated once per capability via [`impl_ndjson_event_subscription`] rather +//! than hand-copied, so the request/parse/`next`/`cancel` logic exists exactly once. `watch` has +//! different (dedup + reconnect) semantics and is not implemented via this macro. +macro_rules! impl_ndjson_event_subscription { + ( + client: $client:ident, + stream: $stream:ident, + event_or_closed: $event_or_closed:ident, + method_fn: $method_fn:ident, + method_name: $method_name:literal, + params: $params_ty:ty, + result: $result_ty:ty, + event: $event_ty:ty, + event_notification: $event_notification_ty:ty, + event_field: $event_field:ident, + closed_notification: $closed_notification_ty:ty, + parse_response_fn: $parse_response_fn:ident, + parse_notification_fn: $parse_notification_fn:ident, + ) => { + #[derive(Clone, Debug, PartialEq)] + pub enum $event_or_closed { + Event($event_ty), + Closed { + reason: openengine_cluster_protocol::SubscriptionCloseReason, + }, + } + + pub struct $client<'a, R, W> { + transport: &'a crate::NdjsonTransport, + } + + impl<'a, R, W> $client<'a, R, W> + where + R: tokio::io::AsyncRead + Send + Unpin + 'static, + W: tokio::io::AsyncWrite + Send + Unpin + 'static, + { + #[must_use] + pub const fn new(transport: &'a crate::NdjsonTransport) -> Self { + Self { transport } + } + + pub async fn $method_fn( + &self, + params: $params_ty, + ) -> Result<($result_ty, $stream<'a, R, W>), crate::ClientError> { + let id = self.transport.next_watch_request_id(); + let request = + serde_json::to_string(&openengine_cluster_protocol::JsonRpcRequest { + jsonrpc: openengine_cluster_protocol::JSON_RPC_VERSION.to_owned(), + id: id.clone(), + method: $method_name.to_owned(), + params, + })?; + let (line, subscription) = self + .transport + .open_subscription(request, id.clone()) + .await?; + let result = $parse_response_fn(&line, &id)?; + let crate::PumpedSubscription { + receiver, + overflowed, + } = subscription.expect(concat!( + "a successful ", + $method_name, + " response must carry a subscriptionId" + )); + let stream = $stream { + transport: self.transport, + receiver, + overflowed, + subscription_id: result.subscription_id.clone(), + }; + Ok((result, stream)) + } + } + + fn $parse_response_fn( + line: &str, + expected_id: &openengine_cluster_protocol::RequestId, + ) -> Result<$result_ty, crate::ClientError> { + let value: serde_json::Value = serde_json::from_str(line) + .map_err(|error| crate::ClientError::InvalidResponse(error.to_string()))?; + if value.get("error").is_some() { + let response: openengine_cluster_protocol::JsonRpcErrorResponse = + serde_json::from_value(value) + .map_err(|error| crate::ClientError::InvalidResponse(error.to_string()))?; + crate::validate_response_identity( + &response.jsonrpc, + response.id.as_ref(), + expected_id, + )?; + return Err(crate::ClientError::Rpc(response.error)); + } + let response: openengine_cluster_protocol::JsonRpcSuccess<$result_ty> = + serde_json::from_value(value) + .map_err(|error| crate::ClientError::InvalidResponse(error.to_string()))?; + crate::validate_response_identity(&response.jsonrpc, Some(&response.id), expected_id)?; + Ok(response.result) + } + + pub struct $stream<'a, R, W> { + transport: &'a crate::NdjsonTransport, + receiver: tokio::sync::mpsc::Receiver, + overflowed: std::sync::Arc, + subscription_id: openengine_cluster_protocol::SubscriptionId, + } + + impl<'a, R, W> $stream<'a, R, W> + where + R: tokio::io::AsyncRead + Send + Unpin + 'static, + W: tokio::io::AsyncWrite + Send + Unpin + 'static, + { + /// Returns the next live event, or a terminal close. Returns `None` once the + /// subscription's channel ends (cancelled locally, or the transport's connection + /// ended). Returns `Some(Err(_))` if a schema-malformed or unexpected-method + /// notification is forwarded for this subscription -- the wire pump routes by + /// subscription id only, so peer-controlled payload shape must never panic here. + pub async fn next(&mut self) -> Option> { + let line = match self.receiver.recv().await { + Some(line) => line, + None if self + .overflowed + .swap(false, std::sync::atomic::Ordering::AcqRel) => + { + return Some(Ok($event_or_closed::Closed { + reason: + openengine_cluster_protocol::SubscriptionCloseReason::SlowConsumer, + })); + } + None => return None, + }; + Some($parse_notification_fn(&line)) + } + + /// Sends `subscription/cancel` for this subscription. Idempotent from the caller's + /// perspective: the server drops an unknown subscription id silently. + pub async fn cancel(&self) -> Result<(), crate::ClientError> { + self.transport + .cancel_subscription(self.subscription_id.clone()) + .await?; + Ok(()) + } + } + + fn $parse_notification_fn(line: &str) -> Result<$event_or_closed, crate::ClientError> { + let value: serde_json::Value = serde_json::from_str(line) + .map_err(|error| crate::ClientError::InvalidResponse(error.to_string()))?; + match value.get("method").and_then(serde_json::Value::as_str) { + Some("event") => { + let notification: openengine_cluster_protocol::JsonRpcNotification< + $event_notification_ty, + > = serde_json::from_value(value) + .map_err(|error| crate::ClientError::InvalidResponse(error.to_string()))?; + Ok($event_or_closed::Event(notification.params.$event_field)) + } + Some("subscription/closed") => { + let notification: openengine_cluster_protocol::JsonRpcNotification< + $closed_notification_ty, + > = serde_json::from_value(value) + .map_err(|error| crate::ClientError::InvalidResponse(error.to_string()))?; + Ok($event_or_closed::Closed { + reason: notification.params.reason, + }) + } + other => Err(crate::ClientError::InvalidResponse(format!( + "unexpected subscription notification method {other:?}" + ))), + } + } + }; +} + +pub(crate) use impl_ndjson_event_subscription; diff --git a/crates/openengine-cluster-client/src/ndjson_watch.rs b/crates/openengine-cluster-client/src/ndjson_watch.rs index 8ca2b634..effc1f61 100644 --- a/crates/openengine-cluster-client/src/ndjson_watch.rs +++ b/crates/openengine-cluster-client/src/ndjson_watch.rs @@ -56,7 +56,7 @@ where let PumpedSubscription { receiver, overflowed, - } = subscription; + } = subscription.expect("a successful watch response must carry a subscriptionId"); let stream = NdjsonReconnectingEventStream { transport: self.transport, receiver, diff --git a/crates/openengine-cluster-client/tests/agent_attach.rs b/crates/openengine-cluster-client/tests/agent_attach.rs new file mode 100644 index 00000000..93789cbf --- /dev/null +++ b/crates/openengine-cluster-client/tests/agent_attach.rs @@ -0,0 +1,154 @@ +//! Client-side NDJSON `agent/attach` subscription coverage: establishment, ordered +//! Working->Output->Settled delivery, cancellation, and unknown/inactive-execution error +//! propagation, driven over the wire against `serve_ndjson`. +//! +//! Generic wire-protocol machinery -- independent request-id sourcing, malformed/unexpected +//! notification handling, and unread-subscription overflow not blocking unary responses -- has no +//! counterpart here: `NdjsonAgentAttachClient`/`NdjsonAgentAttachEventStream` are generated by the +//! exact same [`openengine_cluster_client::ndjson_subscription::impl_ndjson_event_subscription`] +//! macro invocation as `NdjsonLogsClient`/`NdjsonLogsEventStream`, so that machinery is already +//! exhaustively covered once, generically, by `tests/logs.rs`; re-testing it per capability would +//! exercise the exact same generated code a second time. + +use std::sync::Arc; + +use openengine_cluster_client::{ + AgentAttachEventOrClosed, NdjsonAgentAttachClient, NdjsonAgentAttachEventStream, + NdjsonTransport, +}; +use openengine_cluster_protocol::{ + AgentAttachEvent, AgentAttachParams, BoundedAssistantOutput, ExecutionRef, GONE, NOT_FOUND, +}; +use openengine_cluster_server::agent_attach::fixtures::{ + AgentAttachFixtureBackend, AgentAttachFixtureStore, +}; +use openengine_cluster_server::watch::fixtures::{await_ndjson_shutdown, spawn_ndjson}; +use tokio::io::DuplexStream; + +#[path = "cancel_leak_support/mod.rs"] +mod cancel_leak_support; +use cancel_leak_support::assert_cancel_stops_further_delivery; + +fn sample_execution_ref() -> ExecutionRef { + ExecutionRef::new("execution-1").unwrap() +} + +fn agent_attach_params() -> AgentAttachParams { + AgentAttachParams { + execution: sample_execution_ref(), + } +} + +fn sample_output_event(text: &str) -> AgentAttachEvent { + AgentAttachEvent::Output { + text: BoundedAssistantOutput::new(text).unwrap(), + } +} + +/// Awaits the stream's next item and asserts it is the given event, exactly. +async fn expect_next_event( + stream: &mut NdjsonAgentAttachEventStream<'_, DuplexStream, DuplexStream>, + expected: &AgentAttachEvent, +) { + match stream.next().await.unwrap().unwrap() { + AgentAttachEventOrClosed::Event(event) => assert_eq!(&event, expected), + other => panic!("expected an event matching {expected:?}, got {other:?}"), + } +} + +#[tokio::test] +async fn establish_stream_working_output_settled_then_cancel_stops_delivery() { + let store = Arc::new(AgentAttachFixtureStore::new()); + let execution = sample_execution_ref(); + store.register_active(execution.clone()).await; + let (client_write, client_read, server) = + spawn_ndjson(AgentAttachFixtureBackend::new(Arc::clone(&store))); + + let transport = NdjsonTransport::new(client_read, client_write); + let agent_attach_client = NdjsonAgentAttachClient::new(&transport); + let (_result, mut stream) = agent_attach_client + .agent_attach(agent_attach_params()) + .await + .unwrap(); + + for event in [ + AgentAttachEvent::Working {}, + sample_output_event("hello"), + AgentAttachEvent::Settled {}, + ] { + store.publish(&execution, event.clone()).await; + expect_next_event(&mut stream, &event).await; + } + + assert_cancel_stops_further_delivery( + &mut stream, + &transport, + |text| store.publish(&execution, sample_output_event(text)), + |item| match item { + AgentAttachEventOrClosed::Event(AgentAttachEvent::Output { text }) => { + Some(text.as_str().to_owned()) + } + _ => None, + }, + ) + .await; + + drop(stream); + drop(transport); + await_ndjson_shutdown(server).await; +} + +#[tokio::test] +async fn unknown_execution_ref_error_propagates_to_the_client() { + let store = Arc::new(AgentAttachFixtureStore::new()); + let (client_write, client_read, server) = + spawn_ndjson(AgentAttachFixtureBackend::new(Arc::clone(&store))); + + let transport = NdjsonTransport::new(client_read, client_write); + let agent_attach_client = NdjsonAgentAttachClient::new(&transport); + let Err(error) = agent_attach_client + .agent_attach(agent_attach_params()) + .await + else { + panic!("expected an unknown execution ref to be rejected"); + }; + let openengine_cluster_client::ClientError::Rpc(rpc_error) = error else { + panic!("expected an RPC error, got {error}"); + }; + assert_eq!( + rpc_error.data.as_ref().map(|data| data.code.as_str()), + Some(NOT_FOUND) + ); + + drop(transport); + await_ndjson_shutdown(server).await; +} + +#[tokio::test] +async fn inactive_execution_ref_error_propagates_to_the_client() { + let store = Arc::new(AgentAttachFixtureStore::new()); + let execution = sample_execution_ref(); + store.register_active(execution.clone()).await; + store.mark_inactive(&execution).await; + let (client_write, client_read, server) = + spawn_ndjson(AgentAttachFixtureBackend::new(Arc::clone(&store))); + + let transport = NdjsonTransport::new(client_read, client_write); + let agent_attach_client = NdjsonAgentAttachClient::new(&transport); + let Err(error) = agent_attach_client + .agent_attach(agent_attach_params()) + .await + else { + panic!("expected an inactive execution ref to be rejected"); + }; + let openengine_cluster_client::ClientError::Rpc(rpc_error) = error else { + panic!("expected an RPC error, got {error}"); + }; + assert_eq!( + rpc_error.data.as_ref().map(|data| data.code.as_str()), + Some(GONE) + ); + + drop(transport); + await_ndjson_shutdown(server).await; +} diff --git a/crates/openengine-cluster-client/tests/cancel_leak_support/mod.rs b/crates/openengine-cluster-client/tests/cancel_leak_support/mod.rs new file mode 100644 index 00000000..cf18d0ca --- /dev/null +++ b/crates/openengine-cluster-client/tests/cancel_leak_support/mod.rs @@ -0,0 +1,105 @@ +//! Shared "cancel a subscription, then assert the at-most-one-post-cancel-leak model" assertion +//! for `logs`/`agent_attach` NDJSON client tests: `NdjsonLogsEventStream` and +//! `NdjsonAgentAttachEventStream` are both generated by the same +//! `openengine_cluster_client::ndjson_subscription::impl_ndjson_event_subscription` macro +//! invocation and share the same `next`/`cancel` shape, so this exists exactly once instead of +//! being hand-copied per capability. Used by `tests/logs.rs` and `tests/agent_attach.rs`. + +use std::fmt::Debug; +use std::future::Future; +use std::time::Duration; + +use openengine_cluster_client::{ + AgentAttachEventOrClosed, ClientError, ClusterClient, LogEventOrClosed, + NdjsonAgentAttachEventStream, NdjsonLogsEventStream, NdjsonTransport, +}; +use openengine_cluster_protocol::GetParams; +use tokio::io::{AsyncRead, AsyncWrite}; + +/// Minimal shape shared by every generated `Ndjson*EventStream`: `next`/`cancel` with the exact +/// signatures `impl_ndjson_event_subscription!` produces. +pub trait NdjsonEventStream { + type EventOrClosed: Debug; + async fn next(&mut self) -> Option>; + async fn cancel(&self) -> Result<(), ClientError>; +} + +impl NdjsonEventStream for NdjsonLogsEventStream<'_, R, W> +where + R: AsyncRead + Send + Unpin + 'static, + W: AsyncWrite + Send + Unpin + 'static, +{ + type EventOrClosed = LogEventOrClosed; + async fn next(&mut self) -> Option> { + NdjsonLogsEventStream::next(self).await + } + async fn cancel(&self) -> Result<(), ClientError> { + NdjsonLogsEventStream::cancel(self).await + } +} + +impl NdjsonEventStream for NdjsonAgentAttachEventStream<'_, R, W> +where + R: AsyncRead + Send + Unpin + 'static, + W: AsyncWrite + Send + Unpin + 'static, +{ + type EventOrClosed = AgentAttachEventOrClosed; + async fn next(&mut self) -> Option> { + NdjsonAgentAttachEventStream::next(self).await + } + async fn cancel(&self) -> Result<(), ClientError> { + NdjsonAgentAttachEventStream::cancel(self).await + } +} + +/// Cancels `stream`, forces a synchronous unary round trip over `transport` to guarantee the +/// server's read loop has already applied the preceding cancel notification, publishes two +/// further "leak probe" events via `publish`, then asserts the at-most-one-post-cancel-leak model: +/// the server-side subscription task may already have been parked awaiting the next live event at +/// the moment cancellation was processed, so at most one further event may still be delivered +/// before it observes cancellation and stops for good -- and if one is, it must be exactly the +/// first probe (`"maybe-leaked"`), never the second. `classify` maps a delivered event to its +/// probe text; returning `None` for a `Closed` item or any unexpected event variant panics with +/// that item's `Debug` output. +pub async fn assert_cancel_stops_further_delivery( + stream: &mut S, + transport: &NdjsonTransport, + mut publish: impl FnMut(&'static str) -> F, + mut classify: impl FnMut(&S::EventOrClosed) -> Option, +) where + S: NdjsonEventStream, + R: AsyncRead + Send + Unpin + 'static, + W: AsyncWrite + Send + Unpin + 'static, + F: Future, +{ + stream.cancel().await.unwrap(); + ClusterClient::new(transport) + .get(GetParams::default()) + .await + .unwrap(); + + publish("maybe-leaked").await; + publish("must-never-arrive").await; + + let mut leaked = Vec::new(); + loop { + match tokio::time::timeout(Duration::from_millis(300), stream.next()).await { + Ok(Some(Ok(item))) => match classify(&item) { + Some(text) => leaked.push(text), + None => panic!("unexpected notification after cancel: {item:?}"), + }, + Ok(Some(Err(e))) => panic!("unexpected error: {e}"), + Ok(None) | Err(_) => break, + } + } + assert!( + leaked.len() <= 1, + "cancelled subscription received more than one post-cancel event: {leaked:?}" + ); + if let Some(message) = leaked.first() { + assert_eq!( + message, "maybe-leaked", + "cancellation failed to stop delivery before the next published event" + ); + } +} diff --git a/crates/openengine-cluster-client/tests/logs.rs b/crates/openengine-cluster-client/tests/logs.rs index cfdca68e..b3084a9c 100644 --- a/crates/openengine-cluster-client/tests/logs.rs +++ b/crates/openengine-cluster-client/tests/logs.rs @@ -7,17 +7,19 @@ use std::sync::Arc; use std::time::Duration; -use openengine_cluster_client::{ - ClusterClient, JsonRpcTransport, LogEventOrClosed, NdjsonLogsClient, NdjsonTransport, -}; +use openengine_cluster_client::{JsonRpcTransport, LogEventOrClosed, NdjsonLogsClient, NdjsonTransport}; use openengine_cluster_protocol::{ - BoundedLogTarget, BoundedLogMessage, GetParams, LogLevel, LogRecord, LogsParams, + BoundedLogTarget, BoundedLogMessage, LogLevel, LogRecord, LogsParams, }; use openengine_cluster_server::logs::fixtures::{LogsFixtureBackend, LogsFixtureStore}; use openengine_cluster_server::watch::fixtures::{await_ndjson_shutdown, spawn_ndjson}; use serde_json::{json, Value}; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader, DuplexStream}; +#[path = "cancel_leak_support/mod.rs"] +mod cancel_leak_support; +use cancel_leak_support::assert_cancel_stops_further_delivery; + fn sample_log_record(message: &str) -> LogRecord { LogRecord { level: LogLevel::Info, @@ -42,44 +44,16 @@ async fn cancel_stops_further_delivery() { other => panic!("expected an event, got {other:?}"), } - stream.cancel().await.unwrap(); - - // `cancel()` only writes a fire-and-forget notification line -- it does not wait for the - // server to have applied it. Force a synchronous round trip on the same connection so the - // subsequent publishes are guaranteed to happen only after the server's read loop has already - // processed (and synchronously applied) the preceding cancel line. - ClusterClient::new(&transport) - .get(GetParams::default()) - .await - .unwrap(); - - // The server-side subscription task may already be parked awaiting the next live event at the - // moment cancellation is processed, so at most one further event may still be delivered before - // it observes cancellation and stops for good. - store.publish(sample_log_record("maybe-leaked")).await; - store.publish(sample_log_record("must-never-arrive")).await; - - let mut leaked = Vec::new(); - loop { - match tokio::time::timeout(Duration::from_millis(300), stream.next()).await { - Ok(Some(Ok(LogEventOrClosed::Event(record)))) => { - leaked.push(record.message.as_str().to_owned()) - } - Ok(Some(Ok(other))) => panic!("unexpected notification after cancel: {other:?}"), - Ok(Some(Err(e))) => panic!("unexpected error: {e}"), - Ok(None) | Err(_) => break, - } - } - assert!( - leaked.len() <= 1, - "cancelled subscription received more than one post-cancel event: {leaked:?}" - ); - if let Some(message) = leaked.first() { - assert_eq!( - message, "maybe-leaked", - "cancellation failed to stop delivery before the next published event" - ); - } + assert_cancel_stops_further_delivery( + &mut stream, + &transport, + |text| store.publish(sample_log_record(text)), + |item| match item { + LogEventOrClosed::Event(record) => Some(record.message.as_str().to_owned()), + LogEventOrClosed::Closed { .. } => None, + }, + ) + .await; drop(stream); drop(transport); diff --git a/crates/openengine-cluster-protocol/src/agent_attach.rs b/crates/openengine-cluster-protocol/src/agent_attach.rs new file mode 100644 index 00000000..de1e01b7 --- /dev/null +++ b/crates/openengine-cluster-protocol/src/agent_attach.rs @@ -0,0 +1,161 @@ +//! Capability-gated, future-only, read-only `agent/attach` subscription. Scoped to a single bounded +//! opaque [`ExecutionRef`] rather than being cluster-wide. Only a closed `Working`/`Output`/ +//! `Settled` progress algebra is representable: reasoning, tools, provider frames, usage, and +//! session identifiers have no variant. Like [`crate::logs`], this has no run scoping, no replay, +//! and no reconnect: none of `RunId`/`Cursor` appear anywhere in this module's wire types. + +use std::borrow::Cow; + +use schemars::{JsonSchema, Schema, SchemaGenerator}; +use serde::{Deserialize, Serialize}; +use thiserror::Error; + +use crate::wire::{ + impl_bounded_nonempty_string, impl_bounded_redactable_string, impl_validate_gated_wire, +}; +use crate::{SubscriptionCloseReason, SubscriptionId}; + +pub const MAX_EXECUTION_REF_BYTES: usize = 128; +pub const MAX_ASSISTANT_OUTPUT_BYTES: usize = 16_384; +pub const MAX_AGENT_ATTACH_EVENT_ENCODED_BYTES: usize = 65_536; +pub const REDACTED_ASSISTANT_OUTPUT: &str = ""; + +/// A non-empty, bounded, opaque reference to a native execution. Native execution ids are private +/// numeric values; this is deliberately an opaque bounded string rather than exposing storage +/// identity. Bounded by UTF-8 byte length, not character count. +#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] +#[serde(transparent)] +pub struct ExecutionRef(String); + +impl_bounded_nonempty_string!(ExecutionRef, MAX_EXECUTION_REF_BYTES, "128", "ExecutionRef"); + +/// A possibly-empty, bounded, redacted-on-overflow assistant-display output chunk. Bounded by +/// UTF-8 byte length, not character count. This is the only bounded fallback text available for an +/// oversized output -- there is no raw passthrough path anywhere in this module. +#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] +#[serde(transparent)] +pub struct BoundedAssistantOutput(String); + +impl_bounded_redactable_string!( + BoundedAssistantOutput, + MAX_ASSISTANT_OUTPUT_BYTES, + "16384", + "BoundedAssistantOutput", + REDACTED_ASSISTANT_OUTPUT +); + +/// `agent/attach` establishment parameters: the named `{execution}` request. Deliberately closed, +/// rejecting any unknown field. +#[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] +#[serde(deny_unknown_fields, rename_all = "camelCase")] +pub struct AgentAttachParams { + pub execution: ExecutionRef, +} + +/// The `agent/attach` establishment result: only a `subscriptionId`. Deliberately carries no +/// `runId` or `atCursor` -- `agent/attach` is not run-scoped and has no cursor. +#[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] +#[serde(deny_unknown_fields, rename_all = "camelCase")] +pub struct AgentAttachResult { + pub subscription_id: SubscriptionId, +} + +/// The closed public agent-attach progress algebra. This is the only representable shape: +/// reasoning, tools, provider frames, usage, and session identifiers have no variant. `Working` +/// and `Settled` are empty struct variants rather than bare units: serde's internally tagged enum +/// deserialization silently ignores unknown fields on a unit variant regardless of +/// `deny_unknown_fields`, which would otherwise let an unrepresentable field ride along +/// undetected on either of these two variants. +#[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] +#[serde(deny_unknown_fields, tag = "type", rename_all = "snake_case")] +pub enum AgentAttachEvent { + Working {}, + Output { text: BoundedAssistantOutput }, + Settled {}, +} + +#[derive(Clone, Copy, Debug, Error, Eq, PartialEq)] +pub enum AgentAttachEventValidationError { + #[error("agent attach event encoded JSON exceeds {MAX_AGENT_ATTACH_EVENT_ENCODED_BYTES} bytes")] + EncodedTooLarge, +} + +/// Wire body of the generic `event` server notification when carrying an agent-attach progress +/// event. A closed, bounded, validate-gated wire type: both `Serialize` and `Deserialize` run +/// [`AgentAttachEventNotification::validate`], so an oversized encoding can never be produced or +/// accepted on the wire. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct AgentAttachEventNotification { + pub subscription_id: SubscriptionId, + pub event: AgentAttachEvent, +} + +impl AgentAttachEventNotification { + pub fn validate(&self) -> Result<(), AgentAttachEventValidationError> { + let encoded_len = serde_json::to_vec(&AgentAttachEventNotificationRef::from(self)) + .expect("AgentAttachEventNotificationRef fields serialize infallibly") + .len(); + if encoded_len > MAX_AGENT_ATTACH_EVENT_ENCODED_BYTES { + return Err(AgentAttachEventValidationError::EncodedTooLarge); + } + Ok(()) + } +} + +#[derive(Deserialize, JsonSchema)] +#[serde(deny_unknown_fields, rename_all = "camelCase")] +struct AgentAttachEventNotificationWire { + subscription_id: SubscriptionId, + event: AgentAttachEvent, +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct AgentAttachEventNotificationRef<'a> { + subscription_id: &'a SubscriptionId, + event: &'a AgentAttachEvent, +} + +impl From for AgentAttachEventNotification { + fn from(wire: AgentAttachEventNotificationWire) -> Self { + Self { + subscription_id: wire.subscription_id, + event: wire.event, + } + } +} + +impl<'a> From<&'a AgentAttachEventNotification> for AgentAttachEventNotificationRef<'a> { + fn from(notification: &'a AgentAttachEventNotification) -> Self { + Self { + subscription_id: ¬ification.subscription_id, + event: ¬ification.event, + } + } +} + +impl_validate_gated_wire!( + AgentAttachEventNotification, + AgentAttachEventNotificationWire, + AgentAttachEventNotificationRef +); + +impl JsonSchema for AgentAttachEventNotification { + fn schema_name() -> Cow<'static, str> { + "AgentAttachEventNotification".into() + } + + fn json_schema(generator: &mut SchemaGenerator) -> Schema { + generator.subschema_for::() + } +} + +/// Wire body of the terminal `subscription/closed` server notification for an `agent/attach` +/// subscription. Deliberately carries no cursor field -- `agent/attach` gives a type-level +/// "cursorless" guarantee, unlike [`crate::SubscriptionClosedNotification`]. +#[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] +#[serde(deny_unknown_fields, rename_all = "camelCase")] +pub struct AgentAttachClosedNotification { + pub subscription_id: SubscriptionId, + pub reason: SubscriptionCloseReason, +} diff --git a/crates/openengine-cluster-protocol/src/lib.rs b/crates/openengine-cluster-protocol/src/lib.rs index 0c3d4351..4d133756 100644 --- a/crates/openengine-cluster-protocol/src/lib.rs +++ b/crates/openengine-cluster-protocol/src/lib.rs @@ -1,6 +1,7 @@ //! Canonical Open Engine Cluster Protocol wire and domain types. pub mod admission; +pub mod agent_attach; pub mod artifact; pub mod canonical; pub mod diagnostic; @@ -17,6 +18,7 @@ mod wire; pub mod worker; pub use worker::*; +pub use agent_attach::*; pub use artifact::*; pub use admission::*; pub use canonical::*; @@ -203,6 +205,19 @@ pub struct GetResult { pub at_cursor: Option, } +impl GetResult { + /// A `get` result carrying no spec, no cursor, and an empty cluster status. Used by fixture + /// backends that exist only to exercise one specific capability. + #[must_use] + pub const fn empty() -> Self { + Self { + spec: None, + status: ClusterStatus::empty(), + at_cursor: None, + } + } +} + #[derive(Clone, Debug, Default, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] #[serde(deny_unknown_fields, rename_all = "camelCase")] pub struct ServerCapabilities { @@ -211,6 +226,8 @@ pub struct ServerCapabilities { pub graph_profiles: GraphProfileSet, #[serde(default)] pub logs: bool, + #[serde(default)] + pub agent_attach: bool, } #[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] diff --git a/crates/openengine-cluster-protocol/src/logs.rs b/crates/openengine-cluster-protocol/src/logs.rs index 1305bab8..c9156018 100644 --- a/crates/openengine-cluster-protocol/src/logs.rs +++ b/crates/openengine-cluster-protocol/src/logs.rs @@ -4,12 +4,13 @@ use std::borrow::Cow; -use schemars::{json_schema, JsonSchema, Schema, SchemaGenerator}; -use serde::de; -use serde::{Deserialize, Deserializer, Serialize}; +use schemars::{JsonSchema, Schema, SchemaGenerator}; +use serde::{Deserialize, Serialize}; use thiserror::Error; -use crate::wire::impl_validate_gated_wire; +use crate::wire::{ + impl_bounded_nonempty_string, impl_bounded_redactable_string, impl_validate_gated_wire, +}; use crate::{LogLevel, SubscriptionCloseReason, SubscriptionId}; pub const MAX_LOG_TARGET_BYTES: usize = 128; @@ -23,51 +24,12 @@ pub const REDACTED_LOG_MESSAGE: &str = ""; #[serde(transparent)] pub struct BoundedLogTarget(String); -impl BoundedLogTarget { - pub fn new(value: impl Into) -> Result { - let value = value.into(); - if value.is_empty() { - Err("value must not be empty") - } else if value.len() > MAX_LOG_TARGET_BYTES || value.chars().any(char::is_control) { - Err("value must be at most 128 non-control UTF-8 bytes") - } else { - Ok(Self(value)) - } - } - - #[must_use] - pub fn as_str(&self) -> &str { - &self.0 - } -} - -impl<'de> Deserialize<'de> for BoundedLogTarget { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - Self::new(String::deserialize(deserializer)?).map_err(de::Error::custom) - } -} - -impl JsonSchema for BoundedLogTarget { - fn inline_schema() -> bool { - true - } - - fn schema_name() -> Cow<'static, str> { - "BoundedLogTarget".into() - } - - fn json_schema(_generator: &mut SchemaGenerator) -> Schema { - json_schema!({ - "type": "string", - "minLength": 1, - "maxLength": MAX_LOG_TARGET_BYTES, - "pattern": r"^[^\u0000-\u001f\u007f-\u009f]+$" - }) - } -} +impl_bounded_nonempty_string!( + BoundedLogTarget, + MAX_LOG_TARGET_BYTES, + "128", + "BoundedLogTarget" +); /// A possibly-empty, bounded, redacted-on-overflow log message. Bounded by UTF-8 byte length, not /// character count. This is the only bounded fallback text available for an oversized message -- @@ -76,54 +38,13 @@ impl JsonSchema for BoundedLogTarget { #[serde(transparent)] pub struct BoundedLogMessage(String); -impl BoundedLogMessage { - pub fn new(value: impl Into) -> Result { - let value = value.into(); - if value.len() > MAX_LOG_MESSAGE_BYTES || value.chars().any(char::is_control) { - Err("value must be at most 16384 non-control UTF-8 bytes") - } else { - Ok(Self(value)) - } - } - - /// The fixed bounded redaction marker used when a raw message could not be safely projected. - #[must_use] - pub fn redacted() -> Self { - Self(REDACTED_LOG_MESSAGE.to_owned()) - } - - #[must_use] - pub fn as_str(&self) -> &str { - &self.0 - } -} - -impl<'de> Deserialize<'de> for BoundedLogMessage { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - Self::new(String::deserialize(deserializer)?).map_err(de::Error::custom) - } -} - -impl JsonSchema for BoundedLogMessage { - fn inline_schema() -> bool { - true - } - - fn schema_name() -> Cow<'static, str> { - "BoundedLogMessage".into() - } - - fn json_schema(_generator: &mut SchemaGenerator) -> Schema { - json_schema!({ - "type": "string", - "maxLength": MAX_LOG_MESSAGE_BYTES, - "pattern": r"^[^\u0000-\u001f\u007f-\u009f]*$" - }) - } -} +impl_bounded_redactable_string!( + BoundedLogMessage, + MAX_LOG_MESSAGE_BYTES, + "16384", + "BoundedLogMessage", + REDACTED_LOG_MESSAGE +); /// The closed public log record shape: a level, a bounded target, and a bounded (possibly /// redacted) message. No raw bytes, reasoning, tools, credentials, env, or provider/session IDs diff --git a/crates/openengine-cluster-protocol/src/wire.rs b/crates/openengine-cluster-protocol/src/wire.rs index b0016dc2..abe490d2 100644 --- a/crates/openengine-cluster-protocol/src/wire.rs +++ b/crates/openengine-cluster-protocol/src/wire.rs @@ -29,3 +29,128 @@ macro_rules! impl_validate_gated_wire { } pub(crate) use impl_validate_gated_wire; + +/// Shared `new`/`Deserialize`/`JsonSchema` glue for a non-empty, control-character-free string +/// newtype bounded by UTF-8 byte length (not character count). Used by every wire identifier/ +/// target type that must never be empty, so the boilerplate exists exactly once rather than being +/// hand-copied per type. +macro_rules! impl_bounded_nonempty_string { + ($name:ident, $max_bytes:expr, $max_bytes_msg:literal, $schema_name:literal) => { + impl $name { + pub fn new(value: impl Into) -> Result { + let value = value.into(); + if value.is_empty() { + Err("value must not be empty") + } else if value.len() > $max_bytes || value.chars().any(char::is_control) { + Err(concat!( + "value must be at most ", + $max_bytes_msg, + " non-control UTF-8 bytes" + )) + } else { + Ok(Self(value)) + } + } + + #[must_use] + pub fn as_str(&self) -> &str { + &self.0 + } + } + + impl<'de> serde::Deserialize<'de> for $name { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + Self::new(String::deserialize(deserializer)?).map_err(serde::de::Error::custom) + } + } + + impl schemars::JsonSchema for $name { + fn inline_schema() -> bool { + true + } + + fn schema_name() -> std::borrow::Cow<'static, str> { + $schema_name.into() + } + + fn json_schema(_generator: &mut schemars::SchemaGenerator) -> schemars::Schema { + schemars::json_schema!({ + "type": "string", + "minLength": 1, + "maxLength": $max_bytes, + "pattern": r"^[^\u0000-\u001f\u007f-\u009f]+$" + }) + } + } + }; +} + +pub(crate) use impl_bounded_nonempty_string; + +/// Shared `new`/`redacted`/`Deserialize`/`JsonSchema` glue for a possibly-empty, +/// control-character-free, redacted-on-overflow string newtype bounded by UTF-8 byte length (not +/// character count). Used by every wire body-text type whose only bounded fallback for an +/// oversized value is a fixed redaction marker, so the boilerplate exists exactly once rather than +/// being hand-copied per type. +macro_rules! impl_bounded_redactable_string { + ($name:ident, $max_bytes:expr, $max_bytes_msg:literal, $schema_name:literal, $redacted:expr) => { + impl $name { + pub fn new(value: impl Into) -> Result { + let value = value.into(); + if value.len() > $max_bytes || value.chars().any(char::is_control) { + Err(concat!( + "value must be at most ", + $max_bytes_msg, + " non-control UTF-8 bytes" + )) + } else { + Ok(Self(value)) + } + } + + /// The fixed bounded redaction marker used when a raw value could not be safely + /// projected. + #[must_use] + pub fn redacted() -> Self { + Self($redacted.to_owned()) + } + + #[must_use] + pub fn as_str(&self) -> &str { + &self.0 + } + } + + impl<'de> serde::Deserialize<'de> for $name { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + Self::new(String::deserialize(deserializer)?).map_err(serde::de::Error::custom) + } + } + + impl schemars::JsonSchema for $name { + fn inline_schema() -> bool { + true + } + + fn schema_name() -> std::borrow::Cow<'static, str> { + $schema_name.into() + } + + fn json_schema(_generator: &mut schemars::SchemaGenerator) -> schemars::Schema { + schemars::json_schema!({ + "type": "string", + "maxLength": $max_bytes, + "pattern": r"^[^\u0000-\u001f\u007f-\u009f]*$" + }) + } + } + }; +} + +pub(crate) use impl_bounded_redactable_string; diff --git a/crates/openengine-cluster-protocol/tests/agent_attach.rs b/crates/openengine-cluster-protocol/tests/agent_attach.rs new file mode 100644 index 00000000..30e95113 --- /dev/null +++ b/crates/openengine-cluster-protocol/tests/agent_attach.rs @@ -0,0 +1,254 @@ +use openengine_cluster_protocol::{ + AgentAttachClosedNotification, AgentAttachEvent, AgentAttachEventNotification, + AgentAttachParams, AgentAttachResult, BoundedAssistantOutput, ExecutionRef, ServerCapabilities, + SubscriptionCloseReason, SubscriptionId, MAX_AGENT_ATTACH_EVENT_ENCODED_BYTES, + MAX_ASSISTANT_OUTPUT_BYTES, MAX_EXECUTION_REF_BYTES, +}; +use serde_json::json; + +fn sample_execution_ref() -> ExecutionRef { + ExecutionRef::new("execution-1").unwrap() +} + +#[test] +fn agent_attach_params_round_trips_and_rejects_unknown_fields() { + let params = AgentAttachParams { + execution: sample_execution_ref(), + }; + assert_eq!( + serde_json::to_value(¶ms).unwrap(), + json!({ "execution": "execution-1" }) + ); + let round_tripped: AgentAttachParams = + serde_json::from_value(serde_json::to_value(¶ms).unwrap()).unwrap(); + assert_eq!(round_tripped, params); + + assert!(serde_json::from_value::(json!({})).is_err()); + assert!( + serde_json::from_value::(json!({ + "execution": "execution-1", + "unexpected": 1 + })) + .is_err() + ); +} + +#[test] +fn agent_attach_result_round_trips_and_carries_only_a_subscription_id() { + let result = AgentAttachResult { + subscription_id: SubscriptionId::new("sub-1"), + }; + assert_eq!( + serde_json::to_value(&result).unwrap(), + json!({ "subscriptionId": "sub-1" }) + ); + let round_tripped: AgentAttachResult = + serde_json::from_value(serde_json::to_value(&result).unwrap()).unwrap(); + assert_eq!(round_tripped, result); + + assert!( + serde_json::from_value::(json!({ + "subscriptionId": "sub-1", + "runId": "run-1" + })) + .is_err() + ); +} + +#[test] +fn agent_attach_event_round_trips_and_rejects_unknown_fields_for_every_variant() { + let working = AgentAttachEvent::Working {}; + assert_eq!( + serde_json::to_value(&working).unwrap(), + json!({ "type": "working" }) + ); + assert_eq!( + serde_json::from_value::(json!({ "type": "working" })).unwrap(), + working + ); + + let output = AgentAttachEvent::Output { + text: BoundedAssistantOutput::new("hello").unwrap(), + }; + assert_eq!( + serde_json::to_value(&output).unwrap(), + json!({ "type": "output", "text": "hello" }) + ); + assert_eq!( + serde_json::from_value::(json!({ "type": "output", "text": "hello" })) + .unwrap(), + output + ); + + let settled = AgentAttachEvent::Settled {}; + assert_eq!( + serde_json::to_value(&settled).unwrap(), + json!({ "type": "settled" }) + ); + assert_eq!( + serde_json::from_value::(json!({ "type": "settled" })).unwrap(), + settled + ); + + assert!( + serde_json::from_value::(json!({ + "type": "working", + "reasoning": "hidden" + })) + .is_err() + ); + assert!( + serde_json::from_value::(json!({ + "type": "output", + "text": "hello", + "tool": "shell" + })) + .is_err() + ); + assert!(serde_json::from_value::(json!({ "type": "reasoning" })).is_err()); + assert!(serde_json::from_value::(json!({ "type": "tool_call" })).is_err()); + assert!(serde_json::from_value::(json!({ "type": "usage" })).is_err()); + assert!(serde_json::from_value::(json!({ "type": "session_id" })).is_err()); +} + +#[test] +fn agent_attach_event_notification_round_trips_and_carries_only_subscription_id_and_event() { + let notification = AgentAttachEventNotification { + subscription_id: SubscriptionId::new("sub-1"), + event: AgentAttachEvent::Output { + text: BoundedAssistantOutput::new("chunk").unwrap(), + }, + }; + let value = serde_json::to_value(¬ification).unwrap(); + assert_eq!( + value, + json!({ + "subscriptionId": "sub-1", + "event": { "type": "output", "text": "chunk" } + }) + ); + let round_tripped: AgentAttachEventNotification = serde_json::from_value(value).unwrap(); + assert_eq!(round_tripped, notification); + + let mut malformed = serde_json::to_value(¬ification).unwrap(); + malformed["cursor"] = json!("cursor-1"); + assert!(serde_json::from_value::(malformed).is_err()); +} + +#[test] +fn agent_attach_closed_notification_round_trips_and_carries_no_cursor() { + let done = AgentAttachClosedNotification { + subscription_id: SubscriptionId::new("sub-1"), + reason: SubscriptionCloseReason::Done, + }; + assert_eq!( + serde_json::to_value(&done).unwrap(), + json!({ "subscriptionId": "sub-1", "reason": "done" }) + ); + let round_tripped: AgentAttachClosedNotification = + serde_json::from_value(serde_json::to_value(&done).unwrap()).unwrap(); + assert_eq!(round_tripped, done); + + assert!( + serde_json::from_value::(json!({ + "subscriptionId": "sub-1", + "reason": "done", + "lastDeliveredCursor": "cursor-7" + })) + .is_err() + ); +} + +#[test] +fn execution_ref_enforces_byte_length_not_char_count() { + assert!(ExecutionRef::new("").is_err()); + + let exact = "a".repeat(MAX_EXECUTION_REF_BYTES); + assert!(ExecutionRef::new(exact).is_ok()); + + let over = "a".repeat(MAX_EXECUTION_REF_BYTES + 1); + assert!(ExecutionRef::new(over).is_err()); + + // 65 two-byte UTF-8 characters is 130 bytes but only 65 chars: a char-counting bound would + // wrongly accept this. + let multi_byte_over = "\u{00e9}".repeat(65); + assert_eq!(multi_byte_over.chars().count(), 65); + assert_eq!(multi_byte_over.len(), 130); + assert!(ExecutionRef::new(multi_byte_over).is_err()); + + assert!(ExecutionRef::new("has\ncontrol").is_err()); +} + +#[test] +fn bounded_assistant_output_enforces_byte_length_not_char_count() { + assert!(BoundedAssistantOutput::new("").is_ok()); + + let exact = "a".repeat(MAX_ASSISTANT_OUTPUT_BYTES); + assert!(BoundedAssistantOutput::new(exact).is_ok()); + + let over = "a".repeat(MAX_ASSISTANT_OUTPUT_BYTES + 1); + assert!(BoundedAssistantOutput::new(over).is_err()); + + let multi_byte_over = "\u{00e9}".repeat(MAX_ASSISTANT_OUTPUT_BYTES / 2 + 1); + assert!(multi_byte_over.chars().count() <= MAX_ASSISTANT_OUTPUT_BYTES); + assert!(multi_byte_over.len() > MAX_ASSISTANT_OUTPUT_BYTES); + assert!(BoundedAssistantOutput::new(multi_byte_over).is_err()); + + assert!(BoundedAssistantOutput::new("has\tcontrol").is_err()); +} + +#[test] +fn bounded_assistant_output_redacted_marker_is_itself_valid() { + let redacted = BoundedAssistantOutput::redacted(); + assert!(redacted.as_str().len() <= MAX_ASSISTANT_OUTPUT_BYTES); + assert!(!redacted.as_str().chars().any(char::is_control)); +} + +#[test] +fn agent_attach_event_notification_validate_rejects_an_oversized_encoding() { + let within_bounds = AgentAttachEventNotification { + subscription_id: SubscriptionId::new("sub-1"), + event: AgentAttachEvent::Working {}, + }; + assert!(within_bounds.validate().is_ok()); + + // `SubscriptionId` is not itself bounded; a pathologically long one is what actually drives + // the encoded event over `MAX_AGENT_ATTACH_EVENT_ENCODED_BYTES`, since every other field is + // already bounded well under that ceiling on its own. + let oversized = AgentAttachEventNotification { + subscription_id: SubscriptionId::new("s".repeat(MAX_AGENT_ATTACH_EVENT_ENCODED_BYTES)), + event: AgentAttachEvent::Working {}, + }; + assert!(oversized.validate().is_err()); + assert!(serde_json::to_string(&oversized).is_err()); +} + +#[test] +fn server_capabilities_agent_attach_defaults_to_false_and_is_independent_of_logs() { + let value: ServerCapabilities = serde_json::from_value(json!({})).unwrap(); + assert!(!value.agent_attach); + assert!(!value.logs); + + let logs_only = ServerCapabilities { + logs: true, + ..ServerCapabilities::default() + }; + assert!(!logs_only.agent_attach); + + let agent_attach_only = ServerCapabilities { + agent_attach: true, + ..ServerCapabilities::default() + }; + assert!(!agent_attach_only.logs); + + let json = serde_json::to_value(&agent_attach_only).unwrap(); + assert_eq!(json["agentAttach"], json!(true)); + let round_tripped: ServerCapabilities = serde_json::from_value(json).unwrap(); + assert_eq!(round_tripped, agent_attach_only); + + let schema = serde_json::to_value(schemars::schema_for!(ServerCapabilities)).unwrap(); + let validator = jsonschema::validator_for(&schema).unwrap(); + assert!(validator.is_valid(&json!({ "graphProfiles": [], "agentAttach": true }))); + assert!(validator.is_valid(&json!({ "graphProfiles": [], "agentAttach": false }))); + assert!(validator.is_valid(&json!({ "graphProfiles": [] }))); +} diff --git a/crates/openengine-cluster-protocol/tests/capabilities.rs b/crates/openengine-cluster-protocol/tests/capabilities.rs index d1d5b723..4f8e35a8 100644 --- a/crates/openengine-cluster-protocol/tests/capabilities.rs +++ b/crates/openengine-cluster-protocol/tests/capabilities.rs @@ -5,6 +5,7 @@ fn capabilities_of(profiles: Vec) -> ServerCapabilities { ServerCapabilities { graph_profiles: openengine_cluster_protocol::GraphProfileSet::new(profiles).unwrap(), logs: false, + agent_attach: false, } } @@ -12,7 +13,10 @@ fn capabilities_of(profiles: Vec) -> ServerCapabilities { fn empty_capabilities_round_trip() { let value = capabilities_of(vec![]); let json = serde_json::to_value(&value).unwrap(); - assert_eq!(json, json!({ "graphProfiles": [], "logs": false })); + assert_eq!( + json, + json!({ "graphProfiles": [], "logs": false, "agentAttach": false }) + ); let parsed: ServerCapabilities = serde_json::from_value(json).unwrap(); assert_eq!(parsed, value); } @@ -23,7 +27,11 @@ fn single_worker_capabilities_round_trip() { let json = serde_json::to_value(&value).unwrap(); assert_eq!( json, - json!({ "graphProfiles": ["openengine.graph.single-worker/v1"], "logs": false }) + json!({ + "graphProfiles": ["openengine.graph.single-worker/v1"], + "logs": false, + "agentAttach": false + }) ); let parsed: ServerCapabilities = serde_json::from_value(json).unwrap(); assert_eq!(parsed, value); @@ -40,7 +48,8 @@ fn full_v1_capabilities_round_trip() { "openengine.graph.full/v1", "openengine.graph.single-worker/v1" ], - "logs": false + "logs": false, + "agentAttach": false }) ); let parsed: ServerCapabilities = serde_json::from_value(json).unwrap(); diff --git a/crates/openengine-cluster-server/src/agent_attach.rs b/crates/openengine-cluster-server/src/agent_attach.rs new file mode 100644 index 00000000..0d28a608 --- /dev/null +++ b/crates/openengine-cluster-server/src/agent_attach.rs @@ -0,0 +1,99 @@ +//! Transport-neutral, cursorless, future-only agent-attach event streaming and subscription +//! cancellation. Unlike [`crate::logs`], establishing a subscription is fallible: the caller's +//! [`ExecutionRef`](openengine_cluster_protocol::ExecutionRef) must resolve to a live execution. + +pub mod fixtures; +pub mod ports; + +use std::sync::Arc; + +use openengine_cluster_protocol::{ + AgentAttachEvent, AgentAttachParams, AgentAttachResult, ExecutionRef, SubscriptionId, + DEFAULT_SUBSCRIPTION_QUEUE_CAPACITY, GONE, NOT_FOUND, +}; + +pub use ports::{AgentAttachStore, AgentAttachStoreError, AgentAttachSubscription}; + +use crate::subscription_stream::{BoundedEventHandle, BoundedEventStream, BoundedStreamItem}; +use crate::{BackendError, ClusterBackend, Dispatcher}; + +/// One item yielded by [`AgentAttachEventStream`]: either a live progress event, or a terminal +/// slow-consumer close (overflow). Ordinary cancellation (dropping [`AgentAttachHandle`]) yields +/// no `Closed` item -- the stream simply stops. +pub type AgentAttachStreamItem = BoundedStreamItem; + +/// A single bounded live receiver with no buffering or replay -- unlike a durable observation +/// store, `agent/attach` has no retained history to page through. +pub type AgentAttachEventStream = BoundedEventStream; + +/// Drop-to-cancel subscription handle. Cancellation only affects live-subscriber bookkeeping; it +/// never mutates admission or lifecycle cluster state. +pub type AgentAttachHandle = BoundedEventHandle; + +/// Parameters for [`subscribe_and_stream_agent_attach`], grouped to keep that function's argument +/// count reasonable. +pub struct SubscribeAndStreamAgentAttachRequest { + pub execution: ExecutionRef, + pub subscription_id: SubscriptionId, + pub queue_capacity: usize, +} + +/// Canonical [`AgentAttachStoreError`] -> [`BackendError`] mapping: `UnknownExecution` and a +/// wrong-cluster `ExecutionRef` are indistinguishable at the store layer, so both surface as +/// `NOT_FOUND`; `InactiveExecution` surfaces as `GONE`. Neither carries the private execution +/// identity in `details`. Every [`ClusterBackend::agent_attach`] implementation in this crate +/// (production and test fixtures alike) uses this same mapping. +#[must_use] +pub fn default_agent_attach_error_mapping(error: AgentAttachStoreError) -> BackendError { + match error { + AgentAttachStoreError::UnknownExecution => { + BackendError::application(NOT_FOUND, "execution not found", None) + } + AgentAttachStoreError::InactiveExecution => { + BackendError::application(GONE, "execution is no longer active", None) + } + } +} + +/// Establishes a subscription against `store` for `request.execution` and wraps it as an +/// [`AgentAttachEventStream`]. Shared by every [`ClusterBackend::agent_attach`] implementation +/// (production and test fixtures alike); callers supply `map_err` since how an +/// [`AgentAttachStoreError`] maps to a [`BackendError`] is backend-specific in principle, even +/// though every implementation in this crate happens to use +/// [`default_agent_attach_error_mapping`]. +pub async fn subscribe_and_stream_agent_attach( + store: &Arc, + request: SubscribeAndStreamAgentAttachRequest, + map_err: impl FnOnce(AgentAttachStoreError) -> BackendError, +) -> Result<(AgentAttachResult, AgentAttachEventStream, AgentAttachHandle), BackendError> { + let SubscribeAndStreamAgentAttachRequest { + execution, + subscription_id, + queue_capacity, + } = request; + let subscription = store + .subscribe(&execution, queue_capacity) + .await + .map_err(map_err)?; + let result = AgentAttachResult { subscription_id }; + let (stream, handle) = + AgentAttachEventStream::new(subscription.receiver, subscription.overflowed); + Ok((result, stream, handle)) +} + +impl Dispatcher +where + B: ClusterBackend, +{ + /// Non-NDJSON passthrough to the backend's `agent/attach` subscription. NDJSON `agent/attach`/ + /// `subscription/cancel` line framing lives in `stdio/agent_attach.rs`; this only exposes the + /// typed in-process subscription surface. + pub async fn agent_attach( + &self, + params: AgentAttachParams, + ) -> Result<(AgentAttachResult, AgentAttachEventStream, AgentAttachHandle), BackendError> { + self.backend() + .agent_attach(self.context(), params, DEFAULT_SUBSCRIPTION_QUEUE_CAPACITY) + .await + } +} diff --git a/crates/openengine-cluster-server/src/agent_attach/fixtures.rs b/crates/openengine-cluster-server/src/agent_attach/fixtures.rs new file mode 100644 index 00000000..660c7669 --- /dev/null +++ b/crates/openengine-cluster-server/src/agent_attach/fixtures.rs @@ -0,0 +1,167 @@ +//! Minimal `AgentAttachStore`/`ClusterBackend` fixture for exercising the `agent_attach` port +//! contract, independent of `openengine-cluster-testkit`'s production-shaped +//! `InMemoryAdmissionStore`. + +use std::collections::HashMap; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; +use std::sync::Arc; + +use async_trait::async_trait; +use openengine_cluster_protocol::{ + AgentAttachEvent, AgentAttachParams, AgentAttachResult, ClusterStatus, ExecutionRef, GetParams, + GetResult, InitializeParams, InitializeResult, ServerCapabilities, SubscriptionId, +}; +use tokio::sync::{mpsc, Mutex}; + +use super::{ + default_agent_attach_error_mapping, subscribe_and_stream_agent_attach, AgentAttachEventStream, + AgentAttachHandle, AgentAttachStore, AgentAttachStoreError, AgentAttachSubscription, + SubscribeAndStreamAgentAttachRequest, +}; +use crate::{BackendError, ClusterBackend, ConnectionContext}; + +#[derive(Default)] +struct ExecutionState { + active: bool, + live: Vec<(mpsc::Sender, Arc)>, +} + +/// A minimal in-process [`AgentAttachStore`] keyed by [`ExecutionRef`]: registering a ref via +/// [`Self::register_active`] makes it resolvable; [`Self::mark_inactive`] flips it to the `GONE` +/// path without removing it. No retained history: only live fan-out to every currently registered +/// subscriber. A slot whose bounded queue is full is marked overflowed and dropped from live +/// fan-out; a slot whose receiver has already been dropped (cancelled) is dropped silently. +#[derive(Default)] +pub struct AgentAttachFixtureStore { + executions: Mutex>, +} + +impl AgentAttachFixtureStore { + #[must_use] + pub fn new() -> Self { + Self::default() + } + + pub async fn register_active(&self, execution: ExecutionRef) { + self.executions.lock().await.insert( + execution, + ExecutionState { + active: true, + live: Vec::new(), + }, + ); + } + + pub async fn mark_inactive(&self, execution: &ExecutionRef) { + if let Some(state) = self.executions.lock().await.get_mut(execution) { + state.active = false; + } + } + + pub async fn publish(&self, execution: &ExecutionRef, event: AgentAttachEvent) { + let mut executions = self.executions.lock().await; + let Some(state) = executions.get_mut(execution) else { + return; + }; + state.live.retain( + |(sender, overflowed)| match sender.try_send(event.clone()) { + Ok(()) => true, + Err(mpsc::error::TrySendError::Full(_)) => { + overflowed.store(true, Ordering::Release); + false + } + Err(mpsc::error::TrySendError::Closed(_)) => false, + }, + ); + } +} + +#[async_trait] +impl AgentAttachStore for AgentAttachFixtureStore { + async fn subscribe( + &self, + execution: &ExecutionRef, + queue_capacity: usize, + ) -> Result { + let mut executions = self.executions.lock().await; + let state = executions + .get_mut(execution) + .ok_or(AgentAttachStoreError::UnknownExecution)?; + if !state.active { + return Err(AgentAttachStoreError::InactiveExecution); + } + let (sender, receiver) = mpsc::channel(queue_capacity.max(1)); + let overflowed = Arc::new(AtomicBool::new(false)); + state.live.push((sender, Arc::clone(&overflowed))); + Ok(AgentAttachSubscription { + receiver, + overflowed, + }) + } +} + +/// Wraps an [`AgentAttachFixtureStore`] as a minimal [`ClusterBackend`] advertising +/// `agent_attach: true`; `initialize`/`get` return an empty status since this fixture exists only +/// to exercise `agent_attach`. +pub struct AgentAttachFixtureBackend { + pub store: Arc, + next_subscription_id: AtomicU64, +} + +impl AgentAttachFixtureBackend { + #[must_use] + pub fn new(store: Arc) -> Self { + Self { + store, + next_subscription_id: AtomicU64::new(1), + } + } +} + +#[async_trait] +impl ClusterBackend for AgentAttachFixtureBackend { + async fn initialize( + &self, + _context: &ConnectionContext, + _params: InitializeParams, + ) -> Result { + Ok(InitializeResult::new( + ServerCapabilities { + agent_attach: true, + ..ServerCapabilities::default() + }, + ClusterStatus::empty(), + )) + } + + async fn get( + &self, + _context: &ConnectionContext, + _params: GetParams, + ) -> Result { + Ok(GetResult::empty()) + } + + async fn agent_attach( + &self, + _context: &ConnectionContext, + params: AgentAttachParams, + queue_capacity: usize, + ) -> Result<(AgentAttachResult, AgentAttachEventStream, AgentAttachHandle), BackendError> { + let store: Arc = Arc::clone(&self.store) as Arc; + let subscription_id = SubscriptionId::new(format!( + "sub-{}", + self.next_subscription_id.fetch_add(1, Ordering::Relaxed) + )); + subscribe_and_stream_agent_attach( + &store, + SubscribeAndStreamAgentAttachRequest { + execution: params.execution, + subscription_id, + queue_capacity, + }, + default_agent_attach_error_mapping, + ) + .await + } +} diff --git a/crates/openengine-cluster-server/src/agent_attach/ports.rs b/crates/openengine-cluster-server/src/agent_attach/ports.rs new file mode 100644 index 00000000..4342bbcf --- /dev/null +++ b/crates/openengine-cluster-server/src/agent_attach/ports.rs @@ -0,0 +1,37 @@ +//! Transport-neutral, cursorless, future-only agent-attach subscription store contract. + +use std::sync::atomic::AtomicBool; +use std::sync::Arc; + +use async_trait::async_trait; +use openengine_cluster_protocol::{AgentAttachEvent, ExecutionRef}; +use tokio::sync::mpsc; + +/// A freshly registered live agent-attach subscription: a bounded receiver plus the overflow flag +/// the store sets if this subscription's queue ever fills. +pub struct AgentAttachSubscription { + pub receiver: mpsc::Receiver, + pub overflowed: Arc, +} + +/// Errors a store can return when resolving an [`ExecutionRef`]. Unlike [`crate::logs::LogStore`], +/// `agent/attach` is scoped to a single execution that must resolve -- a per-cluster-scoped store +/// cannot and must not distinguish an unknown ref from one that belongs to another cluster, since +/// both must map to the exact same [`UnknownExecution`](AgentAttachStoreError::UnknownExecution) +/// no-leak response. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum AgentAttachStoreError { + UnknownExecution, + InactiveExecution, +} + +/// Backend-neutral, cursorless, future-only agent-attach observation port. There is no retained +/// history to replay: a subscriber only ever observes events published after it registers. +#[async_trait] +pub trait AgentAttachStore: Send + Sync { + async fn subscribe( + &self, + execution: &ExecutionRef, + queue_capacity: usize, + ) -> Result; +} diff --git a/crates/openengine-cluster-server/src/lib.rs b/crates/openengine-cluster-server/src/lib.rs index 8b2656cf..5564cd71 100644 --- a/crates/openengine-cluster-server/src/lib.rs +++ b/crates/openengine-cluster-server/src/lib.rs @@ -1,6 +1,7 @@ //! Backend-neutral Cluster Protocol dispatcher. pub mod admission; +pub mod agent_attach; pub mod graph_verifier; pub mod lifecycle; pub mod logs; @@ -9,6 +10,7 @@ pub mod watch; pub mod worker_registry; mod dispatch; +mod subscription_stream; mod wire; pub(crate) use wire::{serialize_backend_error, serialize_error, serialize_success}; @@ -16,14 +18,15 @@ use std::sync::Arc; use async_trait::async_trait; use openengine_cluster_protocol::{ - ApplyParams, ApplyResult, DeleteParams, DeleteResult, GetParams, GetResult, InitializeParams, - InitializeResult, LogsParams, LogsResult, PlanParams, PlanResult, INVALID_PHASE, - ResubmitParams, ResubmitResult, RetryParams, RetryResult, StopParams, StopResult, UpdateParams, - UpdateResult, WatchParams, WatchResult, + AgentAttachParams, AgentAttachResult, ApplyParams, ApplyResult, DeleteParams, DeleteResult, + GetParams, GetResult, InitializeParams, InitializeResult, LogsParams, LogsResult, PlanParams, + PlanResult, INVALID_PHASE, ResubmitParams, ResubmitResult, RetryParams, RetryResult, + StopParams, StopResult, UpdateParams, UpdateResult, WatchParams, WatchResult, }; use serde_json::Value; use thiserror::Error; +use crate::agent_attach::{AgentAttachEventStream, AgentAttachHandle}; use crate::logs::{LogEventStream, LogsHandle}; use crate::watch::{WatchEventStream, WatchHandle}; @@ -212,6 +215,19 @@ pub trait ClusterBackend: Send + Sync + 'static { None, )) } + + async fn agent_attach( + &self, + _context: &ConnectionContext, + _params: AgentAttachParams, + _queue_capacity: usize, + ) -> Result<(AgentAttachResult, AgentAttachEventStream, AgentAttachHandle), BackendError> { + Err(BackendError::application( + INVALID_PHASE, + "Backend does not support agent attach", + None, + )) + } } pub struct Dispatcher { diff --git a/crates/openengine-cluster-server/src/logs.rs b/crates/openengine-cluster-server/src/logs.rs index 932c605d..c4dbf6f8 100644 --- a/crates/openengine-cluster-server/src/logs.rs +++ b/crates/openengine-cluster-server/src/logs.rs @@ -3,135 +3,29 @@ pub mod fixtures; pub mod ports; -use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use openengine_cluster_protocol::{ - LogRecord, LogsParams, LogsResult, SubscriptionCloseReason, SubscriptionId, - DEFAULT_SUBSCRIPTION_QUEUE_CAPACITY, + LogRecord, LogsParams, LogsResult, SubscriptionId, DEFAULT_SUBSCRIPTION_QUEUE_CAPACITY, }; -use tokio::sync::{mpsc, Notify}; pub use ports::{LogStore, LogSubscription}; +use crate::subscription_stream::{BoundedEventHandle, BoundedEventStream, BoundedStreamItem}; use crate::{BackendError, ClusterBackend, Dispatcher}; /// One item yielded by [`LogEventStream`]: either a live log record, or a terminal slow-consumer /// close (overflow). Ordinary cancellation (dropping [`LogsHandle`]) yields no `Closed` item -- the /// stream simply stops. -#[derive(Clone, Debug, PartialEq)] -pub enum LogStreamItem { - Record(LogRecord), - Closed { reason: SubscriptionCloseReason }, -} - -/// A single bounded live receiver with no buffering or replay -- unlike [`crate::watch::WatchEventStream`], -/// `logs` has no retained history to page through. -pub struct LogEventStream { - receiver: Option>, - overflowed: Arc, - cancelled: Arc, - cancel_notify: Arc, - closed: bool, -} +pub type LogStreamItem = BoundedStreamItem; -impl LogEventStream { - #[must_use] - pub fn new(subscription: LogSubscription) -> (Self, LogsHandle) { - let cancelled = Arc::new(AtomicBool::new(false)); - let cancel_notify = Arc::new(Notify::new()); - let stream = Self { - receiver: Some(subscription.receiver), - overflowed: subscription.overflowed, - cancelled: Arc::clone(&cancelled), - cancel_notify: Arc::clone(&cancel_notify), - closed: false, - }; - (stream, LogsHandle::new(cancelled, cancel_notify)) - } - - /// Returns the next live log record, or a terminal slow-consumer close. Returns `None` once - /// the subscription is cancelled or otherwise permanently done. - pub async fn next(&mut self) -> Option { - if self.closed || self.consume_cancellation() { - return None; - } - self.next_live().await - } - - /// Marks the stream permanently closed if cancellation was requested, returning whether it - /// was. - fn consume_cancellation(&mut self) -> bool { - if !self.cancelled.load(Ordering::Acquire) { - return false; - } - self.receiver = None; - self.closed = true; - true - } - - /// Awaits the next live-delivered record, or a terminal slow-consumer close once the live - /// channel closes with the overflow flag set. - async fn next_live(&mut self) -> Option { - let cancel_notify = Arc::clone(&self.cancel_notify); - let Some(receiver) = self.receiver.as_mut() else { - self.closed = true; - return None; - }; - tokio::select! { - biased; - () = cancel_notify.notified() => { - self.receiver = None; - self.closed = true; - None - } - item = receiver.recv() => match item { - Some(record) => Some(LogStreamItem::Record(record)), - None => { - self.receiver = None; - self.closed = true; - self.overflowed - .load(Ordering::Acquire) - .then_some(LogStreamItem::Closed { - reason: SubscriptionCloseReason::SlowConsumer, - }) - } - }, - } - } -} +/// A single bounded live receiver with no buffering or replay -- unlike +/// [`crate::watch::WatchEventStream`], `logs` has no retained history to page through. +pub type LogEventStream = BoundedEventStream; /// Drop-to-cancel subscription handle. Cancellation only affects live-subscriber bookkeeping; it /// never mutates admission or lifecycle cluster state. -pub struct LogsHandle { - cancelled: Arc, - cancel_notify: Arc, -} - -impl LogsHandle { - fn new(cancelled: Arc, cancel_notify: Arc) -> Self { - Self { - cancelled, - cancel_notify, - } - } - - pub fn cancel(&self) { - self.cancelled.store(true, Ordering::Release); - self.cancel_notify.notify_one(); - } - - #[must_use] - pub fn is_cancelled(&self) -> bool { - self.cancelled.load(Ordering::Acquire) - } -} - -impl Drop for LogsHandle { - fn drop(&mut self) { - self.cancel(); - } -} +pub type LogsHandle = BoundedEventHandle; /// Establishes a subscription against `store` and wraps it as a [`LogEventStream`]. Shared by /// every [`ClusterBackend::logs`] implementation (production and test fixtures alike). Infallible: @@ -143,7 +37,7 @@ pub async fn subscribe_and_stream_logs( ) -> (LogsResult, LogEventStream, LogsHandle) { let subscription = store.subscribe(queue_capacity).await; let result = LogsResult { subscription_id }; - let (stream, handle) = LogEventStream::new(subscription); + let (stream, handle) = LogEventStream::new(subscription.receiver, subscription.overflowed); (result, stream, handle) } diff --git a/crates/openengine-cluster-server/src/stdio.rs b/crates/openengine-cluster-server/src/stdio.rs index 43e19d86..f6a7dc31 100644 --- a/crates/openengine-cluster-server/src/stdio.rs +++ b/crates/openengine-cluster-server/src/stdio.rs @@ -1,8 +1,10 @@ //! NDJSON stdio transport: multiplexes unary JSON-RPC request/response traffic and generic -//! `watch`/`logs` subscription notifications over one bounded-frame connection. +//! `watch`/`logs`/`agent/attach` subscription notifications over one bounded-frame connection. mod admission; +mod agent_attach; mod logs; +mod subscription; use admission::{acquire_task_slot, reject_duplicate, run_writer, InFlightIds, MAX_CONNECTION_TASKS}; @@ -57,6 +59,23 @@ struct ConnectionState { in_flight_ids: InFlightIds, } +/// Races `next` against `cancel`, `biased` toward the cancellation so a `subscription/cancel` that +/// arrives while parked awaiting the next live event wakes the loop immediately instead of only +/// being observed the next time the stream is polled -- which never happens again on an idle run. +/// `biased` also ensures a pending cancellation is never starved by an unbounded run of +/// already-buffered stream items. Shared by `run_watch_subscription` and +/// `subscription::run_bounded_event_subscription`. +pub(super) async fn race_cancel_or_next( + cancel: &Notify, + next: impl std::future::Future>, +) -> Option { + tokio::select! { + biased; + () = cancel.notified() => None, + item = next => item, + } +} + pub async fn serve_ndjson( dispatcher: Dispatcher, reader: R, @@ -162,6 +181,29 @@ where logs::run_logs_subscription(task_dispatcher, id, params, task_state).await; }); } + NdjsonLineKind::AgentAttach { id, params } => { + if reject_duplicate(&in_flight_ids, &outbound_tx, id.clone()).await { + continue; + } + let Some(permit) = + acquire_task_slot(&task_slots, &outbound_tx, Some(id.clone())).await + else { + in_flight_ids.lock().remove(&id); + continue; + }; + let task_dispatcher = dispatcher.clone(); + let task_state = state.clone(); + tasks.spawn(async move { + let _permit = permit; + agent_attach::run_agent_attach_subscription( + task_dispatcher, + id, + params, + task_state, + ) + .await; + }); + } NdjsonLineKind::Passthrough { id } => { if let Some(id) = id.clone() { if reject_duplicate(&in_flight_ids, &outbound_tx, id).await { @@ -221,14 +263,11 @@ async fn run_passthrough_request( } /// Establishes a `watch` subscription and, on success, streams its `event`/`subscription/closed` -/// notifications until the stream ends (overflow, backend close, or cancellation). Registers a -/// per-subscription cancellation [`Notify`] in `subscriptions` for the duration so a concurrent -/// `subscription/cancel` wakes the streaming loop immediately -- even while parked awaiting the -/// next live event -- instead of only being observed the next time `WatchEventStream::next()` is -/// polled, which never happens again on an idle run. The established [`WatchHandle`] is kept alive -/// for the same duration purely to hold its backing flag false: dropping it early would trip -/// `WatchEventStream`'s own cancellation check before anything ever streams. Deregisters once the -/// stream stops. +/// notifications until the stream ends (overflow, backend close, or cancellation), via +/// `subscription::run_established_subscription` -- the same shared establish/loop/cleanup +/// `subscription::run_bounded_event_subscription` uses. The established [`WatchHandle`] is kept +/// alive for the duration purely to hold its backing flag false: dropping it early would trip +/// `WatchEventStream`'s own cancellation check before anything ever streams. async fn run_watch_subscription( dispatcher: Dispatcher, id: RequestId, @@ -244,46 +283,24 @@ async fn run_watch_subscription( } = state; let (response, established) = dispatcher.dispatch_watch(id.clone(), params).await; in_flight_ids.lock().remove(&id); - let Some((subscription_id, mut stream, _handle)) = established else { - let _ = outbound_tx.send(response).await; - return; + let channels = subscription::SubscriptionChannels { + outbound_tx, + subscriptions, }; - let cancel = Arc::new(Notify::new()); - // Register before sending the response: `mpsc::Sender::send` can yield under backpressure - // or Tokio's cooperative-scheduling budget, and a `subscription/cancel` racing in during that - // yield must always find the subscription already cancellable (mirrors the client's - // register-before-resolve ordering in `NdjsonTransport::run_pump`). - subscriptions - .lock() - .insert(subscription_id.clone(), Arc::clone(&cancel)); - if outbound_tx.send(response).await.is_err() { - subscriptions.lock().remove(&subscription_id); + let Some((established, _handle)) = + subscription::establish_subscription(&channels, response, established).await + else { return; - } + }; - loop { - // Race the stream against `cancel` so a `subscription/cancel` that arrives while this task - // is parked inside `next_live`'s `receiver.recv().await` (the steady state for an idle - // subscription) wakes it right away instead of leaking the task and its channel for the - // rest of the connection's lifetime. `biased` checks `cancel` first: without it, once a - // cancellation is pending, `select!`'s default random tie-break could still occasionally - // favor draining more already-buffered stream items over observing the cancellation, - // letting an unbounded number of already-buffered events leak instead of at most the one - // that may already be in flight. - let item = tokio::select! { - biased; - () = cancel.notified() => None, - item = stream.next() => item, - }; - let Some(item) = item else { - break; - }; - let notification = match item { + let encode_subscription_id = established.subscription_id.clone(); + subscription::run_established_subscription(established, channels, move |item| { + Some(match item { WatchStreamItem::Record(record) => serde_json::to_string(&JsonRpcNotification { jsonrpc: JSON_RPC_VERSION.to_owned(), method: "event".to_owned(), params: EventNotification { - subscription_id: subscription_id.clone(), + subscription_id: encode_subscription_id.clone(), run_id: record.run_id, cursor: record.cursor, event: record.event, @@ -297,18 +314,15 @@ async fn run_watch_subscription( jsonrpc: JSON_RPC_VERSION.to_owned(), method: "subscription/closed".to_owned(), params: SubscriptionClosedNotification { - subscription_id: subscription_id.clone(), + subscription_id: encode_subscription_id.clone(), reason, last_delivered_cursor, }, }) .expect("subscription closed notification serialization must succeed"), - }; - if outbound_tx.send(notification).await.is_err() { - break; - } - } - subscriptions.lock().remove(&subscription_id); + }) + }) + .await; } pub async fn serve_stdio(dispatcher: Dispatcher) -> io::Result<()> @@ -374,14 +388,15 @@ where pub(crate) enum NdjsonLineKind { Watch { id: RequestId, params: Value }, Logs { id: RequestId, params: Value }, + AgentAttach { id: RequestId, params: Value }, Cancel(SubscriptionId), Passthrough { id: Option }, } -/// Classifies a decoded NDJSON line without fully deserializing its params: a `watch`/`logs` -/// request is pulled out for subscription handling, a `subscription/cancel` notification is pulled -/// out for inline cancellation, and everything else (including malformed JSON) passes through to -/// [`Dispatcher::dispatch`] unchanged. +/// Classifies a decoded NDJSON line without fully deserializing its params: a `watch`/`logs`/ +/// `agent/attach` request is pulled out for subscription handling, a `subscription/cancel` +/// notification is pulled out for inline cancellation, and everything else (including malformed +/// JSON) passes through to [`Dispatcher::dispatch`] unchanged. pub(crate) fn classify_ndjson_line(line: &str) -> NdjsonLineKind { if let Ok(request) = serde_json::from_str::>(line) { if request.method == "watch" { @@ -396,6 +411,12 @@ pub(crate) fn classify_ndjson_line(line: &str) -> NdjsonLineKind { params: request.params, }; } + if request.method == "agent/attach" { + return NdjsonLineKind::AgentAttach { + id: request.id, + params: request.params, + }; + } return NdjsonLineKind::Passthrough { id: Some(request.id), }; @@ -411,60 +432,4 @@ pub(crate) fn classify_ndjson_line(line: &str) -> NdjsonLineKind { } #[cfg(test)] -mod tests { - use openengine_cluster_protocol::RunId; - - use super::*; - use crate::watch::fixtures::{FixtureBackend, FixtureStore}; - use crate::ConnectionContext; - - /// Regression test for a race where `run_watch_subscription` sent the `watch` response before - /// registering the subscription's `WatchHandle` in `subscriptions`: a `subscription/cancel` - /// processed by the read loop in that window found nothing to remove and the subscription was - /// never cancellable again. Forces the response send to block (a pre-filled, capacity-1 - /// outbound queue that nothing drains) so the task is parked exactly at that send call, then - /// asserts registration has already happened — true only when the insert precedes the send. - #[tokio::test] - async fn subscription_is_registered_before_its_response_send_can_complete() { - let store = Arc::new(FixtureStore::new(RunId::new("run-1"), Vec::new(), 8)); - let dispatcher = Dispatcher::new(FixtureBackend::new(store), ConnectionContext::default()); - - let (outbound_tx, mut outbound_rx) = mpsc::channel::(1); - outbound_tx.send("occupied".to_owned()).await.unwrap(); - - let subscriptions: SubscriptionMap = Arc::new(Mutex::new(HashMap::new())); - let state = ConnectionState { - outbound_tx, - subscriptions: Arc::clone(&subscriptions), - in_flight_ids: Arc::new(Mutex::new(HashSet::new())), - }; - - tokio::spawn(run_watch_subscription( - dispatcher, - RequestId::Integer(1), - Value::Object(serde_json::Map::new()), - state, - )); - - // Let the spawned task run dispatch_watch to completion; it then blocks indefinitely on - // the full outbound queue, since nothing here drains it yet. Poll via bounded cooperative - // yields rather than a fixed sleep: a real-time sleep is a race against however long the - // spawned task actually takes to be scheduled, which flakes under the CPU contention of a - // full `cargo test --workspace` run; yielding is deterministic regardless of load and the - // attempt cap still fails the test if registration never happens. - let mut attempts = 0; - while subscriptions.lock().len() != 1 { - attempts += 1; - assert!( - attempts < 100_000, - "subscription was never registered before its response send could complete, \ - so a cancel racing the response would be lost" - ); - tokio::task::yield_now().await; - } - - // Drain the queue so the parked task can finish instead of leaking past the test. - let _ = outbound_rx.recv().await; - let _ = outbound_rx.recv().await; - } -} +mod tests; diff --git a/crates/openengine-cluster-server/src/stdio/agent_attach.rs b/crates/openengine-cluster-server/src/stdio/agent_attach.rs new file mode 100644 index 00000000..70661fe3 --- /dev/null +++ b/crates/openengine-cluster-server/src/stdio/agent_attach.rs @@ -0,0 +1,101 @@ +//! `agent/attach` subscription NDJSON streaming and dispatch, split out from `stdio.rs` to keep +//! that file's `watch` counterpart readable -- mirrors `run_logs_subscription`/`dispatch_logs` +//! exactly, sharing the same generic `subscriptions` cancellation map and outbound queue, since +//! `agent/attach` reuses the identical wire notification methods and cancellation framing. + +use openengine_cluster_protocol::{ + AgentAttachClosedNotification, AgentAttachEventNotification, AgentAttachParams, + DomainErrorData, JsonRpcNotification, RequestId, SubscriptionId, INVALID_PARAMS, + JSON_RPC_VERSION, SCHEMA_VIOLATION, +}; +use serde_json::Value; + +use super::subscription::{run_bounded_event_subscription, BoundedEventSubscriptionRequest}; +use super::ConnectionState; +use crate::agent_attach::{AgentAttachEventStream, AgentAttachHandle}; +use crate::{serialize_backend_error, serialize_error, serialize_success, ClusterBackend, Dispatcher}; + +/// Establishes an `agent/attach` subscription and, on success, streams its `event`/ +/// `subscription/closed` notifications until the stream ends (overflow, backend close, or +/// cancellation). See `run_logs_subscription`/`run_watch_subscription` for the identical +/// registration-ordering and cancellation-race notes [`run_bounded_event_subscription`] mirrors. +pub(super) async fn run_agent_attach_subscription( + dispatcher: Dispatcher, + id: RequestId, + params: Value, + state: ConnectionState, +) where + B: ClusterBackend, +{ + let (response, established) = dispatcher.dispatch_agent_attach(id.clone(), params).await; + run_bounded_event_subscription( + BoundedEventSubscriptionRequest { + id, + response, + established, + state, + }, + |subscription_id, event| { + serde_json::to_string(&JsonRpcNotification { + jsonrpc: JSON_RPC_VERSION.to_owned(), + method: "event".to_owned(), + params: AgentAttachEventNotification { + subscription_id, + event, + }, + }) + }, + |subscription_id, reason| { + serde_json::to_string(&JsonRpcNotification { + jsonrpc: JSON_RPC_VERSION.to_owned(), + method: "subscription/closed".to_owned(), + params: AgentAttachClosedNotification { + subscription_id, + reason, + }, + }) + }, + ) + .await; +} + +impl Dispatcher +where + B: ClusterBackend, +{ + /// NDJSON-only counterpart to [`Dispatcher::dispatch`] for the `agent/attach` method. Mirrors + /// [`Dispatcher::dispatch_logs`] exactly; only the Rust param/result types differ. + pub(crate) async fn dispatch_agent_attach( + &self, + id: RequestId, + params: Value, + ) -> ( + String, + Option<(SubscriptionId, AgentAttachEventStream, AgentAttachHandle)>, + ) { + let params = match serde_json::from_value::(params) { + Ok(params) => params, + Err(_) => { + return ( + serialize_error( + Some(id), + INVALID_PARAMS, + "Invalid params", + Some(DomainErrorData::new(SCHEMA_VIOLATION)), + ), + None, + ); + } + }; + match self.agent_attach(params).await { + Ok((result, stream, handle)) => { + let subscription_id = result.subscription_id.clone(); + ( + serialize_success(id, result), + Some((subscription_id, stream, handle)), + ) + } + Err(error) => (serialize_backend_error(id, error), None), + } + } +} diff --git a/crates/openengine-cluster-server/src/stdio/logs.rs b/crates/openengine-cluster-server/src/stdio/logs.rs index 08ff188e..9cb3f943 100644 --- a/crates/openengine-cluster-server/src/stdio/logs.rs +++ b/crates/openengine-cluster-server/src/stdio/logs.rs @@ -3,23 +3,21 @@ //! exactly, sharing the same generic `subscriptions` cancellation map and outbound queue, since //! `logs` reuses the identical wire notification methods and cancellation framing. -use std::sync::Arc; - use openengine_cluster_protocol::{ DomainErrorData, JsonRpcNotification, LogEventNotification, LogsClosedNotification, LogsParams, RequestId, SubscriptionId, INVALID_PARAMS, JSON_RPC_VERSION, SCHEMA_VIOLATION, }; use serde_json::Value; -use tokio::sync::Notify; +use super::subscription::{run_bounded_event_subscription, BoundedEventSubscriptionRequest}; use super::ConnectionState; -use crate::logs::{LogEventStream, LogStreamItem, LogsHandle}; +use crate::logs::{LogEventStream, LogsHandle}; use crate::{serialize_backend_error, serialize_error, serialize_success, ClusterBackend, Dispatcher}; /// Establishes a `logs` subscription and, on success, streams its `event`/`subscription/closed` /// notifications until the stream ends (overflow, backend close, or cancellation). See /// `run_watch_subscription` for the identical registration-ordering and cancellation-race notes -/// this mirrors. +/// [`run_bounded_event_subscription`] mirrors. pub(super) async fn run_logs_subscription( dispatcher: Dispatcher, id: RequestId, @@ -28,70 +26,36 @@ pub(super) async fn run_logs_subscription( ) where B: ClusterBackend, { - let ConnectionState { - outbound_tx, - subscriptions, - in_flight_ids, - } = state; let (response, established) = dispatcher.dispatch_logs(id.clone(), params).await; - in_flight_ids.lock().remove(&id); - let Some((subscription_id, mut stream, _handle)) = established else { - let _ = outbound_tx.send(response).await; - return; - }; - let cancel = Arc::new(Notify::new()); - // Register before sending the response: see `run_watch_subscription`'s identical ordering - // note -- a `subscription/cancel` racing in during a backpressured response send must always - // find the subscription already cancellable. - subscriptions - .lock() - .insert(subscription_id.clone(), Arc::clone(&cancel)); - if outbound_tx.send(response).await.is_err() { - subscriptions.lock().remove(&subscription_id); - return; - } - - loop { - // See `run_watch_subscription`'s identical `select!` for why this is `biased` on `cancel`. - let item = tokio::select! { - biased; - () = cancel.notified() => None, - item = stream.next() => item, - }; - let Some(item) = item else { - break; - }; - let encoded = match item { - LogStreamItem::Record(record) => serde_json::to_string(&JsonRpcNotification { + run_bounded_event_subscription( + BoundedEventSubscriptionRequest { + id, + response, + established, + state, + }, + |subscription_id, record| { + serde_json::to_string(&JsonRpcNotification { jsonrpc: JSON_RPC_VERSION.to_owned(), method: "event".to_owned(), params: LogEventNotification { - subscription_id: subscription_id.clone(), + subscription_id, record, }, - }), - LogStreamItem::Closed { reason } => serde_json::to_string(&JsonRpcNotification { + }) + }, + |subscription_id, reason| { + serde_json::to_string(&JsonRpcNotification { jsonrpc: JSON_RPC_VERSION.to_owned(), method: "subscription/closed".to_owned(), params: LogsClosedNotification { - subscription_id: subscription_id.clone(), + subscription_id, reason, }, - }), - }; - // A bounded oversized/unserializable event (e.g. driven by a pathologically large - // backend-supplied subscription id) must never panic the server task -- drop it and end - // only this subscription through the existing cleanup below, never falling back to an - // unbounded or raw wire representation. - let notification = match encoded { - Ok(line) => line, - Err(_) => break, - }; - if outbound_tx.send(notification).await.is_err() { - break; - } - } - subscriptions.lock().remove(&subscription_id); + }) + }, + ) + .await; } impl Dispatcher diff --git a/crates/openengine-cluster-server/src/stdio/subscription.rs b/crates/openengine-cluster-server/src/stdio/subscription.rs new file mode 100644 index 00000000..c540bef9 --- /dev/null +++ b/crates/openengine-cluster-server/src/stdio/subscription.rs @@ -0,0 +1,190 @@ +//! Shared "establish, then forward notifications until the stream ends" machinery for every +//! `serve_ndjson` subscription kind (`watch`, `logs`, `agent_attach`). [`EventSource`] lets +//! [`run_established_subscription`] drive `watch`'s [`crate::watch::WatchEventStream`] and the +//! bounded `logs`/`agent_attach` capabilities' [`BoundedEventStream`] through exactly one +//! establish/loop/cleanup implementation despite their differently-shaped per-item payloads; +//! [`run_bounded_event_subscription`] is the `logs`/`agent_attach`-specific wrapper that also +//! knows how to encode a [`BoundedStreamItem`] into a wire notification. + +use std::sync::Arc; + +use openengine_cluster_protocol::{RequestId, SubscriptionCloseReason, SubscriptionId}; +use tokio::sync::{mpsc, Notify}; + +use super::{race_cancel_or_next, ConnectionState, SubscriptionMap}; +use crate::subscription_stream::{BoundedEventHandle, BoundedEventStream, BoundedStreamItem}; +use crate::watch::{WatchEventStream, WatchStreamItem}; + +/// Minimal event-source abstraction: any stream type with an inherent `async fn next(&mut self) +/// -> Option` can be driven by [`run_established_subscription`]'s one shared loop. +pub(super) trait EventSource { + type Item; + async fn next(&mut self) -> Option; +} + +impl EventSource for BoundedEventStream { + type Item = BoundedStreamItem; + async fn next(&mut self) -> Option { + BoundedEventStream::next(self).await + } +} + +impl EventSource for WatchEventStream { + type Item = WatchStreamItem; + async fn next(&mut self) -> Option { + WatchEventStream::next(self).await + } +} + +/// Grouped owned per-connection channels for [`run_established_subscription`], keeping that +/// function's argument count reasonable. +pub(super) struct SubscriptionChannels { + pub(super) outbound_tx: mpsc::Sender, + pub(super) subscriptions: SubscriptionMap, +} + +/// Grouped identity for an already-established subscription, keeping +/// [`run_established_subscription`]'s argument count reasonable. +pub(super) struct EstablishedSubscription { + pub(super) subscription_id: SubscriptionId, + pub(super) stream: S, + pub(super) cancel: Arc, +} + +/// Registers `subscription_id`'s cancellation [`Notify`] in `subscriptions` before sending +/// `response`: a `subscription/cancel` racing in while the send is backpressured must always find +/// the subscription already cancellable. Rolls the registration back and returns `false` if the +/// send fails, in which case the caller must not stream anything. +async fn register_and_send_established_response( + channels: &SubscriptionChannels, + subscription_id: &SubscriptionId, + cancel: &Arc, + response: String, +) -> bool { + channels + .subscriptions + .lock() + .insert(subscription_id.clone(), Arc::clone(cancel)); + if channels.outbound_tx.send(response).await.is_err() { + channels.subscriptions.lock().remove(subscription_id); + return false; + } + true +} + +/// Shared establishment step for every subscription kind: sends `response`, and if `established` +/// carries a minted subscription, also registers its cancellation [`Notify`] before the send (see +/// [`register_and_send_established_response`]). Returns `None` if establishment failed upstream or +/// the response send failed either way, in which case the caller must not stream anything; the +/// caller must otherwise keep the returned handle alive for the duration of streaming since +/// dropping it early would trip the underlying stream's own cancellation check before anything +/// ever streams. +pub(super) async fn establish_subscription( + channels: &SubscriptionChannels, + response: String, + established: Option<(SubscriptionId, S, H)>, +) -> Option<(EstablishedSubscription, H)> { + let Some((subscription_id, stream, handle)) = established else { + let _ = channels.outbound_tx.send(response).await; + return None; + }; + let cancel = Arc::new(Notify::new()); + if !register_and_send_established_response(channels, &subscription_id, &cancel, response).await + { + return None; + } + Some(( + EstablishedSubscription { + subscription_id, + stream, + cancel, + }, + handle, + )) +} + +/// Streams an already-established subscription's [`EventSource`] until it ends (overflow, backend +/// close, or cancellation) via [`race_cancel_or_next`], encoding each item through `encode`. +/// Returning `None` from `encode` ends the subscription without sending anything -- used to drop +/// an oversized/unserializable item instead of ever falling back to a raw/unbounded wire +/// representation. Deregisters the subscription once the stream stops for any reason. +pub(super) async fn run_established_subscription( + established: EstablishedSubscription, + channels: SubscriptionChannels, + mut encode: impl FnMut(S::Item) -> Option, +) { + let EstablishedSubscription { + subscription_id, + mut stream, + cancel, + } = established; + let SubscriptionChannels { + outbound_tx, + subscriptions, + } = channels; + loop { + let Some(item) = race_cancel_or_next(&cancel, stream.next()).await else { + break; + }; + let Some(notification) = encode(item) else { + break; + }; + if outbound_tx.send(notification).await.is_err() { + break; + } + } + subscriptions.lock().remove(&subscription_id); +} + +/// Grouped arguments for [`run_bounded_event_subscription`], keeping that function's argument +/// count reasonable. +pub(super) struct BoundedEventSubscriptionRequest { + pub(super) id: RequestId, + pub(super) response: String, + pub(super) established: Option<(SubscriptionId, BoundedEventStream, BoundedEventHandle)>, + pub(super) state: ConnectionState, +} + +pub(super) async fn run_bounded_event_subscription( + request: BoundedEventSubscriptionRequest, + encode_event: impl Fn(SubscriptionId, E) -> serde_json::Result, + encode_closed: impl Fn(SubscriptionId, SubscriptionCloseReason) -> serde_json::Result, +) { + let BoundedEventSubscriptionRequest { + id, + response, + established, + state, + } = request; + let ConnectionState { + outbound_tx, + subscriptions, + in_flight_ids, + } = state; + in_flight_ids.lock().remove(&id); + let channels = SubscriptionChannels { + outbound_tx, + subscriptions, + }; + let Some((established, _handle)) = + establish_subscription(&channels, response, established).await + else { + return; + }; + + // A bounded oversized/unserializable event (e.g. driven by a pathologically large + // backend-supplied subscription id) must never panic the server task -- `encode` returning + // `None` below ends only this subscription through the shared cleanup, never falling back to + // an unbounded or raw wire representation. + let encode_subscription_id = established.subscription_id.clone(); + run_established_subscription(established, channels, move |item| { + let encoded = match item { + BoundedStreamItem::Event(event) => encode_event(encode_subscription_id.clone(), event), + BoundedStreamItem::Closed { reason } => { + encode_closed(encode_subscription_id.clone(), reason) + } + }; + encoded.ok() + }) + .await; +} diff --git a/crates/openengine-cluster-server/src/stdio/tests.rs b/crates/openengine-cluster-server/src/stdio/tests.rs new file mode 100644 index 00000000..6e5c1ed1 --- /dev/null +++ b/crates/openengine-cluster-server/src/stdio/tests.rs @@ -0,0 +1,55 @@ +use openengine_cluster_protocol::RunId; + +use super::*; +use crate::watch::fixtures::{FixtureBackend, FixtureStore}; +use crate::ConnectionContext; + +/// Regression test for a race where `run_watch_subscription` sent the `watch` response before +/// registering the subscription's `WatchHandle` in `subscriptions`: a `subscription/cancel` +/// processed by the read loop in that window found nothing to remove and the subscription was +/// never cancellable again. Forces the response send to block (a pre-filled, capacity-1 +/// outbound queue that nothing drains) so the task is parked exactly at that send call, then +/// asserts registration has already happened — true only when the insert precedes the send. +#[tokio::test] +async fn subscription_is_registered_before_its_response_send_can_complete() { + let store = Arc::new(FixtureStore::new(RunId::new("run-1"), Vec::new(), 8)); + let dispatcher = Dispatcher::new(FixtureBackend::new(store), ConnectionContext::default()); + + let (outbound_tx, mut outbound_rx) = mpsc::channel::(1); + outbound_tx.send("occupied".to_owned()).await.unwrap(); + + let subscriptions: SubscriptionMap = Arc::new(Mutex::new(HashMap::new())); + let state = ConnectionState { + outbound_tx, + subscriptions: Arc::clone(&subscriptions), + in_flight_ids: Arc::new(Mutex::new(HashSet::new())), + }; + + tokio::spawn(run_watch_subscription( + dispatcher, + RequestId::Integer(1), + Value::Object(serde_json::Map::new()), + state, + )); + + // Let the spawned task run dispatch_watch to completion; it then blocks indefinitely on + // the full outbound queue, since nothing here drains it yet. Poll via bounded cooperative + // yields rather than a fixed sleep: a real-time sleep is a race against however long the + // spawned task actually takes to be scheduled, which flakes under the CPU contention of a + // full `cargo test --workspace` run; yielding is deterministic regardless of load and the + // attempt cap still fails the test if registration never happens. + let mut attempts = 0; + while subscriptions.lock().len() != 1 { + attempts += 1; + assert!( + attempts < 100_000, + "subscription was never registered before its response send could complete, \ + so a cancel racing the response would be lost" + ); + tokio::task::yield_now().await; + } + + // Drain the queue so the parked task can finish instead of leaking past the test. + let _ = outbound_rx.recv().await; + let _ = outbound_rx.recv().await; +} diff --git a/crates/openengine-cluster-server/src/subscription_stream.rs b/crates/openengine-cluster-server/src/subscription_stream.rs new file mode 100644 index 00000000..ca786570 --- /dev/null +++ b/crates/openengine-cluster-server/src/subscription_stream.rs @@ -0,0 +1,131 @@ +//! Shared "single bounded live receiver, no buffering or replay, drop-to-cancel handle" stream +//! machinery for future-only, overflow-closing subscription capabilities (`logs`, `agent_attach`). +//! Generic over the delivered event type so the `next`/cancellation/overflow race handling exists +//! exactly once instead of being hand-copied per capability. + +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Arc; + +use openengine_cluster_protocol::SubscriptionCloseReason; +use tokio::sync::{mpsc, Notify}; + +/// One item yielded by [`BoundedEventStream`]: either a live event, or a terminal slow-consumer +/// close (overflow). Ordinary cancellation (dropping [`BoundedEventHandle`]) yields no `Closed` +/// item -- the stream simply stops. +#[derive(Clone, Debug, PartialEq)] +pub enum BoundedStreamItem { + Event(E), + Closed { reason: SubscriptionCloseReason }, +} + +/// A single bounded live receiver with no buffering or replay -- unlike +/// [`crate::watch::WatchEventStream`], these capabilities have no retained history to page +/// through. +pub struct BoundedEventStream { + receiver: Option>, + overflowed: Arc, + cancelled: Arc, + cancel_notify: Arc, + closed: bool, +} + +impl BoundedEventStream { + #[must_use] + pub fn new( + receiver: mpsc::Receiver, + overflowed: Arc, + ) -> (Self, BoundedEventHandle) { + let cancelled = Arc::new(AtomicBool::new(false)); + let cancel_notify = Arc::new(Notify::new()); + let stream = Self { + receiver: Some(receiver), + overflowed, + cancelled: Arc::clone(&cancelled), + cancel_notify: Arc::clone(&cancel_notify), + closed: false, + }; + (stream, BoundedEventHandle::new(cancelled, cancel_notify)) + } + + /// Returns the next live event, or a terminal slow-consumer close. Returns `None` once the + /// subscription is cancelled or otherwise permanently done. + pub async fn next(&mut self) -> Option> { + if self.closed || self.consume_cancellation() { + return None; + } + self.next_live().await + } + + /// Marks the stream permanently closed if cancellation was requested, returning whether it + /// was. + fn consume_cancellation(&mut self) -> bool { + if !self.cancelled.load(Ordering::Acquire) { + return false; + } + self.receiver = None; + self.closed = true; + true + } + + /// Awaits the next live-delivered event, or a terminal slow-consumer close once the live + /// channel closes with the overflow flag set. + async fn next_live(&mut self) -> Option> { + let cancel_notify = Arc::clone(&self.cancel_notify); + let Some(receiver) = self.receiver.as_mut() else { + self.closed = true; + return None; + }; + tokio::select! { + biased; + () = cancel_notify.notified() => { + self.receiver = None; + self.closed = true; + None + } + item = receiver.recv() => match item { + Some(event) => Some(BoundedStreamItem::Event(event)), + None => { + self.receiver = None; + self.closed = true; + self.overflowed + .load(Ordering::Acquire) + .then_some(BoundedStreamItem::Closed { + reason: SubscriptionCloseReason::SlowConsumer, + }) + } + }, + } + } +} + +/// Drop-to-cancel subscription handle. Cancellation only affects live-subscriber bookkeeping; it +/// never mutates admission or lifecycle cluster state. +pub struct BoundedEventHandle { + cancelled: Arc, + cancel_notify: Arc, +} + +impl BoundedEventHandle { + fn new(cancelled: Arc, cancel_notify: Arc) -> Self { + Self { + cancelled, + cancel_notify, + } + } + + pub fn cancel(&self) { + self.cancelled.store(true, Ordering::Release); + self.cancel_notify.notify_one(); + } + + #[must_use] + pub fn is_cancelled(&self) -> bool { + self.cancelled.load(Ordering::Acquire) + } +} + +impl Drop for BoundedEventHandle { + fn drop(&mut self) { + self.cancel(); + } +} diff --git a/crates/openengine-cluster-server/tests/agent_attach.rs b/crates/openengine-cluster-server/tests/agent_attach.rs new file mode 100644 index 00000000..9b7e4c85 --- /dev/null +++ b/crates/openengine-cluster-server/tests/agent_attach.rs @@ -0,0 +1,317 @@ +//! Unit-level `AgentAttachStore`/`AgentAttachEventStream` contract tests against a minimal +//! fixture store, independent of the testkit's `InMemoryAdmissionStore`. + +use std::sync::Arc; + +use async_trait::async_trait; +use openengine_cluster_protocol::{ + AgentAttachEvent, AgentAttachParams, AgentAttachResult, BoundedAssistantOutput, ExecutionRef, + GetParams, GetResult, InitializeParams, InitializeResult, ServerCapabilities, SubscriptionId, + GONE, INVALID_PHASE, MAX_AGENT_ATTACH_EVENT_ENCODED_BYTES, NOT_FOUND, +}; +use openengine_cluster_server::agent_attach::fixtures::{ + AgentAttachFixtureBackend, AgentAttachFixtureStore, +}; +use openengine_cluster_server::agent_attach::{ + default_agent_attach_error_mapping, subscribe_and_stream_agent_attach, AgentAttachEventStream, + AgentAttachHandle, AgentAttachStore, AgentAttachStreamItem, + SubscribeAndStreamAgentAttachRequest, +}; +use openengine_cluster_server::watch::fixtures::{await_ndjson_shutdown, spawn_ndjson}; +use openengine_cluster_server::{BackendError, ClusterBackend, ConnectionContext, Dispatcher}; +use serde_json::json; +use tokio::io::BufReader; + +#[path = "capability_default_support/mod.rs"] +mod capability_default_support; +#[path = "ndjson_test_support/mod.rs"] +mod ndjson_test_support; +#[path = "oversized_event_wire_support/mod.rs"] +mod oversized_event_wire_support; +#[path = "oversized_id_backend_support/mod.rs"] +mod oversized_id_backend_support; +use capability_default_support::bare_watch_dispatcher; +use ndjson_test_support::{read_value, request_line, write_line}; +use oversized_event_wire_support::{ + assert_oversized_event_does_not_block_unary_responses, OversizedEventWire, +}; +use oversized_id_backend_support::oversized_id_backend; + +/// An arbitrary, generously large queue capacity for tests that don't care about the exact +/// overflow point. +const AMPLE_CAPACITY: usize = 8; + +fn sample_execution_ref() -> ExecutionRef { + ExecutionRef::new("execution-1").expect("fixture execution ref must be valid") +} + +fn agent_attach_params() -> AgentAttachParams { + AgentAttachParams { + execution: sample_execution_ref(), + } +} + +fn sample_output_event(text: &str) -> AgentAttachEvent { + AgentAttachEvent::Output { + text: BoundedAssistantOutput::new(text).expect("fixture output must be valid"), + } +} + +#[tokio::test] +async fn default_agent_attach_is_unsupported_unless_backend_overrides_it() { + let dispatcher = bare_watch_dispatcher(AMPLE_CAPACITY); + let Err(error) = dispatcher.agent_attach(agent_attach_params()).await else { + panic!("expected the default agent_attach implementation to be unsupported"); + }; + assert_eq!(error.code, INVALID_PHASE); +} + +#[tokio::test] +async fn unknown_execution_ref_returns_not_found_with_no_private_id_in_details() { + let store = Arc::new(AgentAttachFixtureStore::new()); + let dispatcher = Dispatcher::new( + AgentAttachFixtureBackend::new(store), + ConnectionContext::default(), + ); + + let Err(error) = dispatcher.agent_attach(agent_attach_params()).await else { + panic!("expected an unknown execution ref to be rejected"); + }; + assert_eq!(error.code, NOT_FOUND); + assert!(error.details.is_none()); +} + +#[tokio::test] +async fn wrong_cluster_execution_ref_returns_not_found_indistinguishable_from_unknown() { + let store_a = Arc::new(AgentAttachFixtureStore::new()); + let store_b = Arc::new(AgentAttachFixtureStore::new()); + // Minted against store_a's cluster, then resolved against store_b's: a per-cluster-scoped + // store cannot and must not distinguish this from a truly unknown ref. + store_a.register_active(sample_execution_ref()).await; + + let dispatcher_b = Dispatcher::new( + AgentAttachFixtureBackend::new(store_b), + ConnectionContext::default(), + ); + let Err(error) = dispatcher_b.agent_attach(agent_attach_params()).await else { + panic!("expected a wrong-cluster execution ref to be rejected"); + }; + assert_eq!(error.code, NOT_FOUND); + assert!(error.details.is_none()); +} + +#[tokio::test] +async fn inactive_execution_ref_returns_gone() { + let store = Arc::new(AgentAttachFixtureStore::new()); + let execution = sample_execution_ref(); + store.register_active(execution.clone()).await; + store.mark_inactive(&execution).await; + + let dispatcher = Dispatcher::new( + AgentAttachFixtureBackend::new(store), + ConnectionContext::default(), + ); + let Err(error) = dispatcher.agent_attach(agent_attach_params()).await else { + panic!("expected an inactive execution ref to be rejected"); + }; + assert_eq!(error.code, GONE); +} + +#[tokio::test] +async fn agent_attach_streams_only_future_events_no_replay() { + let store = Arc::new(AgentAttachFixtureStore::new()); + let execution = sample_execution_ref(); + store.register_active(execution.clone()).await; + let dispatcher = Dispatcher::new( + AgentAttachFixtureBackend::new(Arc::clone(&store)), + ConnectionContext::default(), + ); + + // Published before the subscription is established: `agent_attach` has no retained history, + // so this must never be observed. + store + .publish(&execution, sample_output_event("before subscribing")) + .await; + + let (_result, mut stream, _handle) = dispatcher + .agent_attach(agent_attach_params()) + .await + .unwrap(); + + store + .publish(&execution, sample_output_event("after subscribing")) + .await; + let item = stream.next().await.unwrap(); + let AgentAttachStreamItem::Event(AgentAttachEvent::Output { text }) = item else { + panic!("expected a live output event"); + }; + assert_eq!(text.as_str(), "after subscribing"); +} + +// `dropping_the_handle_cancels_without_delivering_more_events`, +// `cancelling_wakes_an_already_pending_idle_next_call`, and `queue_overflow_closes_with_slow_consumer` +// have no counterpart here: `AgentAttachEventStream`/`AgentAttachHandle` are plain type aliases for +// `crate::subscription_stream`'s generic `BoundedEventStream`/`BoundedEventHandle` (see +// `agent_attach.rs`), so those behaviors are already exhaustively covered once, generically, by +// `tests/logs.rs`'s identical aliases over `LogRecord`; re-testing them per `E` would exercise the +// exact same shared code path a second time. + +// An `agent_attach`-only backend whose subscription id is deliberately pathologically large -- +// large enough on its own to push `AgentAttachEventNotification`'s encoded size over +// `MAX_AGENT_ATTACH_EVENT_ENCODED_BYTES`, even though every other field is already bounded well +// under that ceiling. Delegates `initialize`/`get` to a wrapped `AgentAttachFixtureBackend` and +// overrides only `agent_attach`. +oversized_id_backend! { + name: OversizedIdAgentAttachBackend, + inner: AgentAttachFixtureBackend, + method: agent_attach, + params: AgentAttachParams, + result: AgentAttachResult, + stream: AgentAttachEventStream, + handle: AgentAttachHandle, + body: |self, params, queue_capacity| { + let store: Arc = + Arc::clone(&self.inner.store) as Arc; + let subscription_id = SubscriptionId::new("s".repeat(MAX_AGENT_ATTACH_EVENT_ENCODED_BYTES)); + subscribe_and_stream_agent_attach( + &store, + SubscribeAndStreamAgentAttachRequest { + execution: params.execution, + subscription_id, + queue_capacity, + }, + default_agent_attach_error_mapping, + ) + .await + }, +} + +#[tokio::test] +async fn oversized_event_encoding_ends_only_that_subscription_without_panicking() { + let store = Arc::new(AgentAttachFixtureStore::new()); + let execution = sample_execution_ref(); + store.register_active(execution.clone()).await; + let (mut write, read, server) = spawn_ndjson(OversizedIdAgentAttachBackend { + inner: AgentAttachFixtureBackend::new(Arc::clone(&store)), + }); + let mut read = BufReader::new(read); + + // Encodes to well over `MAX_AGENT_ATTACH_EVENT_ENCODED_BYTES` purely because of the backend's + // pathologically large subscription id; the notification loop must drop it silently instead + // of panicking the server task. + assert_oversized_event_does_not_block_unary_responses( + OversizedEventWire { + write: &mut write, + read: &mut read, + }, + "agent/attach", + json!({ "execution": "execution-1" }), + || store.publish(&execution, sample_output_event("won't fit")), + ) + .await; + + drop(write); + await_ndjson_shutdown(server).await; +} + +#[tokio::test] +async fn agent_attach_capability_toggle_does_not_alter_durable_fold_or_execution() { + let store = Arc::new(AgentAttachFixtureStore::new()); + let enabled_backend = AgentAttachFixtureBackend::new(Arc::clone(&store)); + let enabled = enabled_backend + .initialize( + &ConnectionContext::default(), + InitializeParams { + protocol_version: openengine_cluster_protocol::PROTOCOL_VERSION.to_owned(), + }, + ) + .await + .unwrap(); + assert!(enabled.capabilities.agent_attach); + assert_eq!( + enabled.status, + openengine_cluster_protocol::ClusterStatus::empty() + ); + + struct DisabledBackend; + #[async_trait] + impl ClusterBackend for DisabledBackend { + async fn initialize( + &self, + _context: &ConnectionContext, + _params: InitializeParams, + ) -> Result { + Ok(InitializeResult::new( + ServerCapabilities::default(), + openengine_cluster_protocol::ClusterStatus::empty(), + )) + } + + async fn get( + &self, + _context: &ConnectionContext, + _params: GetParams, + ) -> Result { + Ok(GetResult::empty()) + } + } + let disabled = DisabledBackend + .initialize( + &ConnectionContext::default(), + InitializeParams { + protocol_version: openengine_cluster_protocol::PROTOCOL_VERSION.to_owned(), + }, + ) + .await + .unwrap(); + assert!(!disabled.capabilities.agent_attach); + // Toggling agent_attach changes only the advertised capability flag; both backends report the + // exact same empty cluster status/lifecycle for identical get() requests. + assert_eq!(enabled.status, disabled.status); +} + +#[tokio::test] +async fn subscription_cancel_is_sole_post_establishment_operation() { + let store = Arc::new(AgentAttachFixtureStore::new()); + let execution = sample_execution_ref(); + store.register_active(execution.clone()).await; + let (mut write, read, server) = + spawn_ndjson(AgentAttachFixtureBackend::new(Arc::clone(&store))); + let mut read = BufReader::new(read); + + write_line( + &mut write, + &request_line(1, "agent/attach", json!({ "execution": "execution-1" })), + ) + .await; + let established = read_value(&mut read).await; + let subscription_id = established["result"]["subscriptionId"] + .as_str() + .expect("agent/attach must establish a subscription") + .to_owned(); + + store + .publish(&execution, sample_output_event("hello")) + .await; + let event = read_value(&mut read).await; + assert_eq!(event["method"], "event"); + + write_line( + &mut write, + &json!({ + "jsonrpc": "2.0", + "method": "subscription/cancel", + "params": { "subscriptionId": subscription_id } + }) + .to_string(), + ) + .await; + + // A synchronous round trip proves the cancel notification was already processed. + write_line(&mut write, &request_line(2, "get", json!({}))).await; + let get_response = read_value(&mut read).await; + assert_eq!(get_response["id"], 2); + + drop(write); + await_ndjson_shutdown(server).await; +} diff --git a/crates/openengine-cluster-server/tests/capability_default_support/mod.rs b/crates/openengine-cluster-server/tests/capability_default_support/mod.rs new file mode 100644 index 00000000..b8019914 --- /dev/null +++ b/crates/openengine-cluster-server/tests/capability_default_support/mod.rs @@ -0,0 +1,20 @@ +//! Shared default-dispatcher constructor for `logs`/`agent_attach` tests that exercise their +//! `ClusterBackend` default (capability-disabled) implementation. + +use std::sync::Arc; + +use openengine_cluster_protocol::RunId; +use openengine_cluster_server::watch::fixtures::{FixtureBackend, FixtureStore}; +use openengine_cluster_server::{ConnectionContext, Dispatcher}; + +/// A [`Dispatcher`] over the `watch`-only [`FixtureBackend`], used by `logs`/`agent_attach` tests +/// to exercise their `ClusterBackend` default (capability-disabled) implementation: this backend +/// overrides neither, so any call falls straight through to the trait's `INVALID_PHASE` default. +pub fn bare_watch_dispatcher(queue_capacity: usize) -> Dispatcher { + let store = Arc::new(FixtureStore::new( + RunId::new("run-1"), + Vec::new(), + queue_capacity, + )); + Dispatcher::new(FixtureBackend::new(store), ConnectionContext::default()) +} diff --git a/crates/openengine-cluster-server/tests/initialize_capabilities.rs b/crates/openengine-cluster-server/tests/initialize_capabilities.rs index bc22c1e9..551acee8 100644 --- a/crates/openengine-cluster-server/tests/initialize_capabilities.rs +++ b/crates/openengine-cluster-server/tests/initialize_capabilities.rs @@ -43,6 +43,7 @@ impl ClusterBackend for SingleWorkerCapabilitiesBackend { ServerCapabilities { graph_profiles: GraphProfileSet::new(vec![GraphProfile::SingleWorker]).unwrap(), logs: false, + agent_attach: false, }, ClusterStatus::empty(), )) diff --git a/crates/openengine-cluster-server/tests/logs.rs b/crates/openengine-cluster-server/tests/logs.rs index 029a187f..33cea169 100644 --- a/crates/openengine-cluster-server/tests/logs.rs +++ b/crates/openengine-cluster-server/tests/logs.rs @@ -4,26 +4,30 @@ use std::sync::Arc; use std::time::Duration; -use async_trait::async_trait; use openengine_cluster_protocol::{ - BoundedLogTarget, BoundedLogMessage, GetParams, GetResult, InitializeParams, InitializeResult, - LogLevel, LogRecord, LogsParams, LogsResult, SubscriptionCloseReason, SubscriptionId, - INVALID_PHASE, MAX_LOG_EVENT_ENCODED_BYTES, + BoundedLogTarget, BoundedLogMessage, LogLevel, LogRecord, LogsParams, SubscriptionCloseReason, + SubscriptionId, INVALID_PHASE, MAX_LOG_EVENT_ENCODED_BYTES, }; use openengine_cluster_server::logs::fixtures::{LogsFixtureBackend, LogsFixtureStore}; -use openengine_cluster_server::logs::{ - subscribe_and_stream_logs, LogEventStream, LogStore, LogStreamItem, LogsHandle, -}; -use openengine_cluster_server::watch::fixtures::FixtureBackend as WatchFixtureBackend; -use openengine_cluster_server::watch::fixtures::FixtureStore as WatchFixtureStore; +use openengine_cluster_server::logs::{subscribe_and_stream_logs, LogStore, LogStreamItem}; use openengine_cluster_server::watch::fixtures::{await_ndjson_shutdown, spawn_ndjson}; -use openengine_cluster_server::{BackendError, ClusterBackend, ConnectionContext, Dispatcher}; +use openengine_cluster_server::{ClusterBackend, ConnectionContext, Dispatcher}; use serde_json::json; use tokio::io::BufReader; +#[path = "capability_default_support/mod.rs"] +mod capability_default_support; #[path = "ndjson_test_support/mod.rs"] mod ndjson_test_support; -use ndjson_test_support::{read_value, request_line, write_line}; +#[path = "oversized_event_wire_support/mod.rs"] +mod oversized_event_wire_support; +#[path = "oversized_id_backend_support/mod.rs"] +mod oversized_id_backend_support; +use capability_default_support::bare_watch_dispatcher; +use oversized_event_wire_support::{ + assert_oversized_event_does_not_block_unary_responses, OversizedEventWire, +}; +use oversized_id_backend_support::oversized_id_backend; /// Every test below either doesn't care about the exact overflow point or drives it through this /// fixed capacity directly against the backend; only @@ -41,15 +45,7 @@ fn sample_log_record(message: &str) -> LogRecord { #[tokio::test] async fn default_logs_is_unsupported_unless_the_backend_overrides_it() { - let store = Arc::new(WatchFixtureStore::new( - openengine_cluster_protocol::RunId::new("run-1"), - Vec::new(), - AMPLE_CAPACITY, - )); - let dispatcher = Dispatcher::new( - WatchFixtureBackend::new(store), - ConnectionContext::default(), - ); + let dispatcher = bare_watch_dispatcher(AMPLE_CAPACITY); let Err(error) = dispatcher.logs(LogsParams::default()).await else { panic!("expected the default logs implementation to be unsupported"); }; @@ -72,7 +68,7 @@ async fn logs_streams_only_future_records_no_replay() { store.publish(sample_log_record("after subscribing")).await; let item = stream.next().await.unwrap(); - let LogStreamItem::Record(record) = item else { + let LogStreamItem::Event(record) = item else { panic!("expected a live log record"); }; assert_eq!(record.message.as_str(), "after subscribing"); @@ -128,7 +124,7 @@ async fn queue_overflow_closes_with_slow_consumer_and_carries_no_cursor() { store.publish(sample_log_record("second")).await; let first = stream.next().await.unwrap(); - let LogStreamItem::Record(record) = first else { + let LogStreamItem::Event(record) = first else { panic!("expected the first buffered record"); }; assert_eq!(record.message.as_str(), "first"); @@ -142,43 +138,24 @@ async fn queue_overflow_closes_with_slow_consumer_and_carries_no_cursor() { ); } -/// A `logs`-only backend whose subscription id is deliberately pathologically large -- large -/// enough on its own to push `LogEventNotification`'s encoded size over -/// `MAX_LOG_EVENT_ENCODED_BYTES`, even though every `LogRecord` field is already bounded well -/// under that ceiling. Delegates `initialize`/`get` to a wrapped [`LogsFixtureBackend`] and -/// overrides only `logs`. -struct OversizedIdLogsBackend { +// A `logs`-only backend whose subscription id is deliberately pathologically large -- large +// enough on its own to push `LogEventNotification`'s encoded size over +// `MAX_LOG_EVENT_ENCODED_BYTES`, even though every `LogRecord` field is already bounded well +// under that ceiling. Delegates `initialize`/`get` to a wrapped `LogsFixtureBackend` and +// overrides only `logs`. +oversized_id_backend! { + name: OversizedIdLogsBackend, inner: LogsFixtureBackend, -} - -#[async_trait] -impl ClusterBackend for OversizedIdLogsBackend { - async fn initialize( - &self, - context: &ConnectionContext, - params: InitializeParams, - ) -> Result { - self.inner.initialize(context, params).await - } - - async fn get( - &self, - context: &ConnectionContext, - params: GetParams, - ) -> Result { - self.inner.get(context, params).await - } - - async fn logs( - &self, - _context: &ConnectionContext, - _params: LogsParams, - queue_capacity: usize, - ) -> Result<(LogsResult, LogEventStream, LogsHandle), BackendError> { + method: logs, + params: LogsParams, + result: openengine_cluster_protocol::LogsResult, + stream: openengine_cluster_server::logs::LogEventStream, + handle: openengine_cluster_server::logs::LogsHandle, + body: |self, _params, queue_capacity| { let store: Arc = Arc::clone(&self.inner.store) as Arc; let subscription_id = SubscriptionId::new("s".repeat(MAX_LOG_EVENT_ENCODED_BYTES)); Ok(subscribe_and_stream_logs(&store, subscription_id, queue_capacity).await) - } + }, } #[tokio::test] @@ -189,22 +166,19 @@ async fn oversized_event_encoding_ends_only_that_subscription_without_panicking( }); let mut read = BufReader::new(read); - write_line(&mut write, &request_line(1, "logs", json!({}))).await; - let established = read_value(&mut read).await; - assert!(established.get("result").is_some(), "{established}"); - // Encodes to well over `MAX_LOG_EVENT_ENCODED_BYTES` purely because of the backend's // pathologically large subscription id; the notification loop must drop it silently instead // of panicking the server task. - store.publish(sample_log_record("won't fit")).await; - - write_line(&mut write, &request_line(2, "get", json!({}))).await; - let get_response = read_value(&mut read).await; - assert_eq!(get_response["id"], 2); - assert!( - get_response.get("result").is_some(), - "connection must keep serving unary requests after an unencodable event: {get_response}" - ); + assert_oversized_event_does_not_block_unary_responses( + OversizedEventWire { + write: &mut write, + read: &mut read, + }, + "logs", + json!({}), + || store.publish(sample_log_record("won't fit")), + ) + .await; drop(write); await_ndjson_shutdown(server).await; diff --git a/crates/openengine-cluster-server/tests/ndjson_test_support/mod.rs b/crates/openengine-cluster-server/tests/ndjson_test_support/mod.rs index 1eacd21d..a603f981 100644 --- a/crates/openengine-cluster-server/tests/ndjson_test_support/mod.rs +++ b/crates/openengine-cluster-server/tests/ndjson_test_support/mod.rs @@ -1,6 +1,6 @@ //! Shared raw NDJSON line I/O primitives for driving `serve_ndjson` directly over -//! `tokio::io::duplex` pipes without a typed protocol client. Used by `tests/subscription_ndjson.rs` -//! and `tests/logs.rs`. +//! `tokio::io::duplex` pipes without a typed protocol client. Used by `tests/subscription_ndjson.rs`, +//! `tests/logs.rs`, and `tests/agent_attach.rs`. use serde_json::{json, Value}; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader, DuplexStream}; diff --git a/crates/openengine-cluster-server/tests/oversized_event_wire_support/mod.rs b/crates/openengine-cluster-server/tests/oversized_event_wire_support/mod.rs new file mode 100644 index 00000000..27cef1da --- /dev/null +++ b/crates/openengine-cluster-server/tests/oversized_event_wire_support/mod.rs @@ -0,0 +1,48 @@ +//! Shared wire-level assertion for "an oversized/unencodable subscription event ends its own +//! subscription without panicking the server task, and the connection keeps serving unary +//! requests afterward" -- `logs` and `agent_attach` need byte-for-byte the same establish/publish/ +//! assert shape over `serve_ndjson`, differing only in the establishment method/params and how the +//! oversized event is published, so it exists exactly once instead of being hand-copied per +//! capability. Used by `tests/logs.rs` and `tests/agent_attach.rs`. + +use std::future::Future; + +use serde_json::{json, Value}; +use tokio::io::{BufReader, DuplexStream}; + +use super::ndjson_test_support::{read_value, request_line, write_line}; + +/// Grouped connection halves for [`assert_oversized_event_does_not_block_unary_responses`], +/// keeping that function's argument count reasonable. +pub struct OversizedEventWire<'a> { + pub write: &'a mut DuplexStream, + pub read: &'a mut BufReader, +} + +/// Establishes a subscription over `wire` by sending `method`/`params`, publishes an oversized +/// event via `publish_oversized`, then asserts the connection still answers a unary `get` +/// afterward -- proving the oversized/unencodable event was dropped instead of ever reaching the +/// wire or panicking the server task. +pub async fn assert_oversized_event_does_not_block_unary_responses( + wire: OversizedEventWire<'_>, + method: &str, + params: Value, + publish_oversized: impl FnOnce() -> F, +) where + F: Future, +{ + let OversizedEventWire { write, read } = wire; + write_line(write, &request_line(1, method, params)).await; + let established = read_value(read).await; + assert!(established.get("result").is_some(), "{established}"); + + publish_oversized().await; + + write_line(write, &request_line(2, "get", json!({}))).await; + let get_response = read_value(read).await; + assert_eq!(get_response["id"], 2); + assert!( + get_response.get("result").is_some(), + "connection must keep serving unary requests after an unencodable event: {get_response}" + ); +} diff --git a/crates/openengine-cluster-server/tests/oversized_id_backend_support/mod.rs b/crates/openengine-cluster-server/tests/oversized_id_backend_support/mod.rs new file mode 100644 index 00000000..b9b5fea0 --- /dev/null +++ b/crates/openengine-cluster-server/tests/oversized_id_backend_support/mod.rs @@ -0,0 +1,62 @@ +//! Generates the `ClusterBackend` wrapper used only to test that an oversized/unencodable +//! subscription event ends its own subscription without panicking the server task. `logs` and +//! `agent_attach` need byte-for-byte the same "delegate `initialize`/`get` to a fixture backend, +//! override only the capability under test to mint a pathologically large subscription id" shape, +//! so it is generated once via this macro (mirroring +//! `openengine_cluster_client::ndjson_subscription::impl_ndjson_event_subscription`'s +//! generate-once-per-capability approach) rather than hand-copied per capability. Used by +//! `tests/logs.rs` and `tests/agent_attach.rs`. +macro_rules! oversized_id_backend { + ( + name: $name:ident, + inner: $inner_ty:ty, + method: $method:ident, + params: $params_ty:ty, + result: $result_ty:ty, + stream: $stream_ty:ty, + handle: $handle_ty:ty, + body: |$self_:ident, $params_ident:ident, $queue_capacity_ident:ident| $body:block, + ) => { + struct $name { + inner: $inner_ty, + } + + #[async_trait::async_trait] + impl openengine_cluster_server::ClusterBackend for $name { + async fn initialize( + &self, + context: &openengine_cluster_server::ConnectionContext, + params: openengine_cluster_protocol::InitializeParams, + ) -> Result< + openengine_cluster_protocol::InitializeResult, + openengine_cluster_server::BackendError, + > { + self.inner.initialize(context, params).await + } + + async fn get( + &self, + context: &openengine_cluster_server::ConnectionContext, + params: openengine_cluster_protocol::GetParams, + ) -> Result< + openengine_cluster_protocol::GetResult, + openengine_cluster_server::BackendError, + > { + self.inner.get(context, params).await + } + + async fn $method( + &$self_, + _context: &openengine_cluster_server::ConnectionContext, + $params_ident: $params_ty, + $queue_capacity_ident: usize, + ) -> Result< + ($result_ty, $stream_ty, $handle_ty), + openengine_cluster_server::BackendError, + > + $body + } + }; +} + +pub(crate) use oversized_id_backend; diff --git a/crates/openengine-cluster-testkit/src/agent_attach.rs b/crates/openengine-cluster-testkit/src/agent_attach.rs new file mode 100644 index 00000000..e7ab85e2 --- /dev/null +++ b/crates/openengine-cluster-testkit/src/agent_attach.rs @@ -0,0 +1,7 @@ +//! Deterministic in-memory `AgentAttachStore` keyed by `ExecutionRef`. This is a testkit fixture, +//! not a production execution registry: there is no retained history, only live fan-out to every +//! currently registered subscriber for a resolvable execution. Reuses +//! [`openengine_cluster_server::agent_attach::fixtures::AgentAttachFixtureStore`] rather than +//! re-implementing the same in-memory fan-out store a second time. + +pub use openengine_cluster_server::agent_attach::fixtures::AgentAttachFixtureStore as InMemoryAgentAttachStore; diff --git a/crates/openengine-cluster-testkit/src/agent_attach_artifacts.rs b/crates/openengine-cluster-testkit/src/agent_attach_artifacts.rs new file mode 100644 index 00000000..f07bb5d8 --- /dev/null +++ b/crates/openengine-cluster-testkit/src/agent_attach_artifacts.rs @@ -0,0 +1,124 @@ +//! Generated `agent_attach` golden fixtures. `agent-attach-session.json` is produced by driving a +//! minimal real dispatcher configured with an [`InMemoryAgentAttachStore`] through an +//! `agent/attach` subscription and recording every event an actual subscriber receives; the +//! remaining fixtures document standalone wire shapes for request/close framing that no single +//! session exercises. Unlike `logs_artifacts.rs`, this does not run through +//! `AdmissionCoordinator`: `agent_attach`'s `ExecutionRef` resolution has no production backend +//! yet (`#686` owns the native producer/adapter), so this reuses the server crate's minimal +//! `agent_attach: true` fixture backend instead. + +use std::sync::Arc; + +use openengine_cluster_protocol::{ + AgentAttachClosedNotification, AgentAttachEvent, AgentAttachEventNotification, + AgentAttachParams, BoundedAssistantOutput, ExecutionRef, SubscriptionCloseReason, + SubscriptionId, +}; +use openengine_cluster_server::agent_attach::fixtures::AgentAttachFixtureBackend; +use openengine_cluster_server::agent_attach::AgentAttachStreamItem; +use openengine_cluster_server::{ConnectionContext, Dispatcher}; +use serde_json::json; + +use crate::agent_attach::InMemoryAgentAttachStore; +use crate::artifacts::{json_artifact, Artifact}; + +const ROOT: &str = "protocol/openengine-cluster/v1"; + +pub(crate) async fn generate_agent_attach_goldens() -> Vec { + vec![ + json_artifact( + format!("{ROOT}/goldens/agent-attach-session.json"), + json!(agent_attach_session().await), + ), + json_artifact( + format!("{ROOT}/fixtures/agent_attach/agent-attach-params.json"), + json!(AgentAttachParams { + execution: sample_execution_ref(), + }), + ), + json_artifact( + format!("{ROOT}/fixtures/agent_attach/agent-attach-closed.json"), + json!([ + AgentAttachClosedNotification { + subscription_id: SubscriptionId::new("sub-1"), + reason: SubscriptionCloseReason::Done, + }, + AgentAttachClosedNotification { + subscription_id: SubscriptionId::new("sub-2"), + reason: SubscriptionCloseReason::SlowConsumer, + }, + ]), + ), + json_artifact( + format!("{ROOT}/fixtures/agent_attach/agent-attach-event.json"), + json!([ + AgentAttachEvent::Working {}, + sample_output_event("agent dispatch started"), + redacted_output_event(), + AgentAttachEvent::Settled {}, + ]), + ), + ] +} + +fn sample_execution_ref() -> ExecutionRef { + ExecutionRef::new("execution-1").expect("fixture execution ref must be valid") +} + +fn sample_output_event(text: &str) -> AgentAttachEvent { + AgentAttachEvent::Output { + text: BoundedAssistantOutput::new(text).expect("fixture output must be valid"), + } +} + +fn redacted_output_event() -> AgentAttachEvent { + AgentAttachEvent::Output { + text: BoundedAssistantOutput::redacted(), + } +} + +/// Establishes an `agent/attach` subscription against a real dispatcher configured with an +/// [`InMemoryAgentAttachStore`], publishes a small deterministic sequence of sample events +/// (including one built via `BoundedAssistantOutput::redacted()`), and returns every +/// `AgentAttachEventNotification` an actual subscriber receives. Reuses +/// [`AgentAttachFixtureBackend`] (the same minimal `agent_attach: true` backend the server crate's +/// own fixture tests exercise) rather than wiring up a second one here. +async fn agent_attach_session() -> Vec { + let store = Arc::new(InMemoryAgentAttachStore::default()); + let execution = sample_execution_ref(); + store.register_active(execution.clone()).await; + let backend = AgentAttachFixtureBackend::new(Arc::clone(&store)); + let dispatcher = Dispatcher::new(backend, ConnectionContext::default()); + + let (result, mut stream, _handle) = dispatcher + .agent_attach(AgentAttachParams { + execution: execution.clone(), + }) + .await + .expect("a backend configured with an agent attach store must support agent_attach"); + + store + .publish(&execution, AgentAttachEvent::Working {}) + .await; + store + .publish(&execution, sample_output_event("agent dispatch started")) + .await; + store.publish(&execution, redacted_output_event()).await; + store + .publish(&execution, AgentAttachEvent::Settled {}) + .await; + + let mut notifications = Vec::new(); + while notifications.len() < 4 { + match stream.next().await { + Some(AgentAttachStreamItem::Event(event)) => { + notifications.push(AgentAttachEventNotification { + subscription_id: result.subscription_id.clone(), + event, + }); + } + _ => break, + } + } + notifications +} diff --git a/crates/openengine-cluster-testkit/src/artifacts.rs b/crates/openengine-cluster-testkit/src/artifacts.rs index 4ef74e92..fc71cc22 100644 --- a/crates/openengine-cluster-testkit/src/artifacts.rs +++ b/crates/openengine-cluster-testkit/src/artifacts.rs @@ -2,13 +2,14 @@ use std::fs; use std::path::{Path, PathBuf}; use openengine_cluster_protocol::{ - ApplyParams, ApplyResult, ArtifactRef, CompiledGraphIr, DeleteParams, DeleteResult, - EventNotification, GetParams, GetResult, GraphDiagnostic, GraphSpec, InitializeParams, - InitializeResult, JsonRpcNotification, JsonRpcRequest, JsonRpcResponse, LogEventNotification, - LogsClosedNotification, LogsParams, LogsResult, PlanParams, PlanResult, ResubmitParams, - ResubmitResult, RetryParams, RetryResult, StopParams, StopResult, StructuralBounds, - SubscriptionCancelParams, SubscriptionClosedNotification, UpdateParams, UpdateResult, - WatchParams, WatchResult, + AgentAttachClosedNotification, AgentAttachEventNotification, AgentAttachParams, + AgentAttachResult, ApplyParams, ApplyResult, ArtifactRef, CompiledGraphIr, DeleteParams, + DeleteResult, EventNotification, GetParams, GetResult, GraphDiagnostic, GraphSpec, + InitializeParams, InitializeResult, JsonRpcNotification, JsonRpcRequest, JsonRpcResponse, + LogEventNotification, LogsClosedNotification, LogsParams, LogsResult, PlanParams, PlanResult, + ResubmitParams, ResubmitResult, RetryParams, RetryResult, StopParams, StopResult, + StructuralBounds, SubscriptionCancelParams, SubscriptionClosedNotification, UpdateParams, + UpdateResult, WatchParams, WatchResult, }; use openengine_cluster_server::{ConnectionContext, Dispatcher}; use schemars::{schema_for, JsonSchema}; @@ -72,6 +73,10 @@ pub struct ImplementedProtocolSchema { pub logs_response: JsonRpcResponse, pub log_event_notification: JsonRpcNotification, pub logs_closed_notification: JsonRpcNotification, + pub agent_attach_request: JsonRpcRequest, + pub agent_attach_response: JsonRpcResponse, + pub agent_attach_event_notification: JsonRpcNotification, + pub agent_attach_closed_notification: JsonRpcNotification, } pub async fn generate_artifacts() -> Vec { @@ -134,6 +139,7 @@ pub async fn generate_artifacts() -> Vec { artifacts.extend(crate::lifecycle_artifacts::generate_delete_goldens().await); artifacts.extend(crate::watch_artifacts::generate_watch_goldens().await); artifacts.extend(crate::logs_artifacts::generate_logs_goldens().await); + artifacts.extend(crate::agent_attach_artifacts::generate_agent_attach_goldens().await); for (name, request) in cases { let response = dispatcher.dispatch(request).await; artifacts.push(Artifact { diff --git a/crates/openengine-cluster-testkit/src/artifacts/openrpc.rs b/crates/openengine-cluster-testkit/src/artifacts/openrpc.rs index dd94d23b..3ba5ebcf 100644 --- a/crates/openengine-cluster-testkit/src/artifacts/openrpc.rs +++ b/crates/openengine-cluster-testkit/src/artifacts/openrpc.rs @@ -1,5 +1,6 @@ use openengine_cluster_protocol::{ - ApplyParams, DeleteParams, ResubmitParams, RetryParams, StopParams, UpdateParams, + AgentAttachParams, ApplyParams, DeleteParams, ResubmitParams, RetryParams, StopParams, + UpdateParams, }; use schemars::schema_for; use serde_json::{json, Value}; @@ -23,6 +24,7 @@ pub(super) fn document() -> Value { get_method(), watch_method(), logs_method(), + agent_attach_method(), ], "components": { "schemas": { @@ -34,10 +36,12 @@ pub(super) fn document() -> Value { } }, "x-generic-subscription-framing": { - "description": "watch and logs each establish a subscription via one normal JSON-RPC \ - result; subsequent delivery uses the generic notification methods below, shared by \ - every subscription-based method. There is no watch/event, watch/cancel, \ - watch/closed, logs/event, logs/cancel, or logs/closed method on the wire.", + "description": "watch, logs, and agent/attach each establish a subscription via one \ + normal JSON-RPC result; subsequent delivery uses the generic notification methods \ + below, shared by every subscription-based method. There is no watch/event, \ + watch/cancel, watch/closed, logs/event, logs/cancel, logs/closed, \ + agent/attach/event, agent/attach/cancel, or agent/attach/closed method on the \ + wire.", "notifications": { "event": { "$ref": "schema.json#/$defs/EventNotification" }, "subscription/cancel": { "$ref": "schema.json#/$defs/SubscriptionCancelParams" }, @@ -269,3 +273,24 @@ fn logs_method() -> Value { } }) } + +fn agent_attach_method() -> Value { + // `ExecutionRef` is inline-schema (like `logs`'s `BoundedLogTarget`/`BoundedLogMessage`), so it + // has no standalone `$defs` entry to `$ref` -- extract its actual inline schema from a + // generated `AgentAttachParams` schema instead of hand-authoring a `$ref` that would dangle. + let schema = serde_json::to_value(schema_for!(AgentAttachParams)) + .expect("agent_attach parameter JSON Schema serialization must succeed"); + json!({ + "name": "agent/attach", + "paramStructure": "by-name", + "params": [{ + "name": "execution", + "required": true, + "schema": property_schema(&schema, "execution") + }], + "result": { + "name": "agentAttachResult", + "schema": { "$ref": "schema.json#/$defs/AgentAttachResult" } + } + }) +} diff --git a/crates/openengine-cluster-testkit/src/capability_vectors.rs b/crates/openengine-cluster-testkit/src/capability_vectors.rs index 49dd2f96..e9c00a85 100644 --- a/crates/openengine-cluster-testkit/src/capability_vectors.rs +++ b/crates/openengine-cluster-testkit/src/capability_vectors.rs @@ -21,3 +21,13 @@ pub fn assert_logs_capability(capabilities: &ServerCapabilities, expected: bool) "advertised logs capability did not match expected value" ); } + +/// Asserts the advertised `agentAttach` capability flag matches `expected`. No backend-to-capability +/// registry lives here: callers supply the expectation, so a scripted vector here makes no claim +/// about any production backend. +pub fn assert_agent_attach_capability(capabilities: &ServerCapabilities, expected: bool) { + assert_eq!( + capabilities.agent_attach, expected, + "advertised agent_attach capability did not match expected value" + ); +} diff --git a/crates/openengine-cluster-testkit/src/lib.rs b/crates/openengine-cluster-testkit/src/lib.rs index f8f7af9c..8fc048a0 100644 --- a/crates/openengine-cluster-testkit/src/lib.rs +++ b/crates/openengine-cluster-testkit/src/lib.rs @@ -2,6 +2,8 @@ pub mod admission; mod admission_artifacts; +pub mod agent_attach; +mod agent_attach_artifacts; pub mod artifacts; pub mod capability_vectors; pub mod fixture; diff --git a/crates/openengine-cluster-testkit/src/logs_artifacts.rs b/crates/openengine-cluster-testkit/src/logs_artifacts.rs index b3b1b1de..47e2f095 100644 --- a/crates/openengine-cluster-testkit/src/logs_artifacts.rs +++ b/crates/openengine-cluster-testkit/src/logs_artifacts.rs @@ -102,7 +102,7 @@ async fn logs_session() -> Vec { let mut notifications = Vec::new(); while notifications.len() < 3 { match stream.next().await { - Some(LogStreamItem::Record(record)) => { + Some(LogStreamItem::Event(record)) => { notifications.push(LogEventNotification { subscription_id: result.subscription_id.clone(), record, diff --git a/crates/openengine-cluster-testkit/tests/agent_attach.rs b/crates/openengine-cluster-testkit/tests/agent_attach.rs new file mode 100644 index 00000000..461026e7 --- /dev/null +++ b/crates/openengine-cluster-testkit/tests/agent_attach.rs @@ -0,0 +1,89 @@ +use openengine_cluster_protocol::{AgentAttachEventNotification, GraphProfileSet, ServerCapabilities}; +use openengine_cluster_testkit::capability_vectors::assert_agent_attach_capability; + +#[path = "schema_support/mod.rs"] +mod schema_support; +use schema_support::find_schema; + +#[test] +fn agent_attach_capability_vector_matches_server_capabilities() { + let disabled = ServerCapabilities { + graph_profiles: GraphProfileSet::new(vec![]).unwrap(), + logs: false, + agent_attach: false, + }; + assert_agent_attach_capability(&disabled, false); + + let enabled = ServerCapabilities { + graph_profiles: GraphProfileSet::new(vec![]).unwrap(), + logs: false, + agent_attach: true, + }; + assert_agent_attach_capability(&enabled, true); +} + +#[tokio::test] +async fn generated_agent_attach_goldens_validate_against_the_published_schema() { + let artifacts = openengine_cluster_testkit::artifacts::generate_artifacts().await; + let schema = find_schema(&artifacts); + let mut event_schema = schema["$defs"]["AgentAttachEventNotification"].clone(); + event_schema["$defs"] = schema["$defs"].clone(); + let event_validator = jsonschema::validator_for(&event_schema).unwrap(); + + let session = artifacts + .iter() + .find(|artifact| { + artifact + .relative_path + .ends_with("/goldens/agent-attach-session.json") + }) + .unwrap(); + let notifications: Vec = + serde_json::from_slice(&session.bytes).unwrap(); + assert!(!notifications.is_empty()); + for notification in ¬ifications { + let value = serde_json::to_value(notification).unwrap(); + assert!( + event_validator.is_valid(&value), + "generated agent attach event notification failed schema validation: {value}" + ); + } +} + +#[tokio::test] +async fn generated_agent_attach_fixtures_validate_against_the_published_schema() { + let artifacts = openengine_cluster_testkit::artifacts::generate_artifacts().await; + let schema = find_schema(&artifacts); + + let checks = [ + ("agent-attach-params.json", "AgentAttachParams"), + ("agent-attach-closed.json", "AgentAttachClosedNotification"), + ("agent-attach-event.json", "AgentAttachEvent"), + ]; + for (file_name, def_name) in checks { + let mut def_schema = schema["$defs"][def_name].clone(); + assert!( + !def_schema.is_null(), + "schema.json is missing $defs/{def_name}" + ); + def_schema["$defs"] = schema["$defs"].clone(); + let validator = jsonschema::validator_for(&def_schema).unwrap(); + + let artifact = artifacts + .iter() + .find(|artifact| { + artifact + .relative_path + .ends_with(&format!("/fixtures/agent_attach/{file_name}")) + }) + .unwrap_or_else(|| panic!("missing fixtures/agent_attach/{file_name}")); + let value: serde_json::Value = serde_json::from_slice(&artifact.bytes).unwrap(); + let values = value.as_array().cloned().unwrap_or_else(|| vec![value]); + for value in values { + assert!( + validator.is_valid(&value), + "{file_name} entry failed schema validation against {def_name}: {value}" + ); + } + } +} diff --git a/crates/openengine-cluster-testkit/tests/artifacts.rs b/crates/openengine-cluster-testkit/tests/artifacts.rs index cead790b..04c00fe2 100644 --- a/crates/openengine-cluster-testkit/tests/artifacts.rs +++ b/crates/openengine-cluster-testkit/tests/artifacts.rs @@ -7,6 +7,10 @@ use openengine_cluster_testkit::artifacts::{ ArtifactError, check_artifacts, generate_artifacts, write_artifacts, }; +#[path = "schema_support/mod.rs"] +mod schema_support; +use schema_support::find_schema; + #[tokio::test] async fn generated_artifacts_are_complete_and_committed_without_drift() { let artifacts = generate_artifacts().await; @@ -132,7 +136,8 @@ async fn openrpc_exposes_only_the_implemented_protocol_methods() { "delete", "get", "watch", - "logs" + "logs", + "agent/attach" ] ); for component in [ @@ -261,11 +266,7 @@ async fn openrpc_exposes_closed_lifecycle_controls() { #[tokio::test] async fn schema_is_derived_from_canonical_envelopes_with_required_success_ids() { let artifacts = generate_artifacts().await; - let schema = artifacts - .iter() - .find(|artifact| artifact.relative_path.ends_with("/schema.json")) - .unwrap(); - let schema: serde_json::Value = serde_json::from_slice(&schema.bytes).unwrap(); + let schema = find_schema(&artifacts); let definitions = schema["$defs"].as_object().unwrap(); assert!( diff --git a/crates/openengine-cluster-testkit/tests/capability_vectors.rs b/crates/openengine-cluster-testkit/tests/capability_vectors.rs index 37bf47e5..342fcd18 100644 --- a/crates/openengine-cluster-testkit/tests/capability_vectors.rs +++ b/crates/openengine-cluster-testkit/tests/capability_vectors.rs @@ -5,6 +5,7 @@ fn capabilities_of(profiles: Vec) -> ServerCapabilities { ServerCapabilities { graph_profiles: GraphProfileSet::new(profiles).unwrap(), logs: false, + agent_attach: false, } } diff --git a/crates/openengine-cluster-testkit/tests/logs.rs b/crates/openengine-cluster-testkit/tests/logs.rs index 59a9eaa2..c1b35551 100644 --- a/crates/openengine-cluster-testkit/tests/logs.rs +++ b/crates/openengine-cluster-testkit/tests/logs.rs @@ -10,6 +10,10 @@ use openengine_cluster_testkit::admission::{InMemoryAdmissionStore, ScriptedVeri use openengine_cluster_testkit::capability_vectors::assert_logs_capability; use openengine_cluster_testkit::logs::InMemoryLogStore; +#[path = "schema_support/mod.rs"] +mod schema_support; +use schema_support::find_schema; + fn initialize_params() -> InitializeParams { InitializeParams { protocol_version: PROTOCOL_VERSION.to_owned(), @@ -47,12 +51,14 @@ fn logs_capability_vector_matches_server_capabilities() { let disabled = ServerCapabilities { graph_profiles: GraphProfileSet::new(vec![]).unwrap(), logs: false, + agent_attach: false, }; assert_logs_capability(&disabled, false); let enabled = ServerCapabilities { graph_profiles: GraphProfileSet::new(vec![]).unwrap(), logs: true, + agent_attach: false, }; assert_logs_capability(&enabled, true); } @@ -60,11 +66,7 @@ fn logs_capability_vector_matches_server_capabilities() { #[tokio::test] async fn generated_logs_goldens_validate_against_the_published_schema() { let artifacts = openengine_cluster_testkit::artifacts::generate_artifacts().await; - let schema = artifacts - .iter() - .find(|artifact| artifact.relative_path.ends_with("/schema.json")) - .unwrap(); - let schema: serde_json::Value = serde_json::from_slice(&schema.bytes).unwrap(); + let schema = find_schema(&artifacts); let mut event_schema = schema["$defs"]["LogEventNotification"].clone(); event_schema["$defs"] = schema["$defs"].clone(); let event_validator = jsonschema::validator_for(&event_schema).unwrap(); diff --git a/crates/openengine-cluster-testkit/tests/protocol_v1.rs b/crates/openengine-cluster-testkit/tests/protocol_v1.rs index aae346f5..1648b59e 100644 --- a/crates/openengine-cluster-testkit/tests/protocol_v1.rs +++ b/crates/openengine-cluster-testkit/tests/protocol_v1.rs @@ -40,7 +40,7 @@ fn canonical_empty_results_have_exact_wire_shape() { serde_json::to_value(initialize).unwrap(), serde_json::json!({ "protocolVersion": "openengine.cluster/v1", - "capabilities": { "graphProfiles": [], "logs": false }, + "capabilities": { "graphProfiles": [], "logs": false, "agentAttach": false }, "status": { "phase": "empty", "observedGeneration": null, diff --git a/crates/openengine-cluster-testkit/tests/schema_support/mod.rs b/crates/openengine-cluster-testkit/tests/schema_support/mod.rs new file mode 100644 index 00000000..82fb6b6b --- /dev/null +++ b/crates/openengine-cluster-testkit/tests/schema_support/mod.rs @@ -0,0 +1,13 @@ +//! Shared "pull `schema.json` out of a generated artifact set and parse it" helper for tests that +//! validate golden/fixture payloads against the published schema. Used by `tests/artifacts.rs`, +//! `tests/logs.rs`, and `tests/agent_attach.rs`. + +use openengine_cluster_testkit::artifacts::Artifact; + +pub fn find_schema(artifacts: &[Artifact]) -> serde_json::Value { + let schema = artifacts + .iter() + .find(|artifact| artifact.relative_path.ends_with("/schema.json")) + .unwrap(); + serde_json::from_slice(&schema.bytes).unwrap() +} diff --git a/protocol/openengine-cluster/v1/fixtures/agent_attach/agent-attach-closed.json b/protocol/openengine-cluster/v1/fixtures/agent_attach/agent-attach-closed.json new file mode 100644 index 00000000..c604555a --- /dev/null +++ b/protocol/openengine-cluster/v1/fixtures/agent_attach/agent-attach-closed.json @@ -0,0 +1,10 @@ +[ + { + "reason": "done", + "subscriptionId": "sub-1" + }, + { + "reason": "SLOW_CONSUMER", + "subscriptionId": "sub-2" + } +] diff --git a/protocol/openengine-cluster/v1/fixtures/agent_attach/agent-attach-event.json b/protocol/openengine-cluster/v1/fixtures/agent_attach/agent-attach-event.json new file mode 100644 index 00000000..a68cd1dd --- /dev/null +++ b/protocol/openengine-cluster/v1/fixtures/agent_attach/agent-attach-event.json @@ -0,0 +1,16 @@ +[ + { + "type": "working" + }, + { + "text": "agent dispatch started", + "type": "output" + }, + { + "text": "", + "type": "output" + }, + { + "type": "settled" + } +] diff --git a/protocol/openengine-cluster/v1/fixtures/agent_attach/agent-attach-params.json b/protocol/openengine-cluster/v1/fixtures/agent_attach/agent-attach-params.json new file mode 100644 index 00000000..f4d1a302 --- /dev/null +++ b/protocol/openengine-cluster/v1/fixtures/agent_attach/agent-attach-params.json @@ -0,0 +1,3 @@ +{ + "execution": "execution-1" +} diff --git a/protocol/openengine-cluster/v1/goldens/admission-lifecycle.ndjson b/protocol/openengine-cluster/v1/goldens/admission-lifecycle.ndjson index 58a80531..ee0a0019 100644 --- a/protocol/openengine-cluster/v1/goldens/admission-lifecycle.ndjson +++ b/protocol/openengine-cluster/v1/goldens/admission-lifecycle.ndjson @@ -1,5 +1,5 @@ {"id":"admission-init","jsonrpc":"2.0","method":"initialize","params":{"protocolVersion":"openengine.cluster/v1"}} -{"jsonrpc":"2.0","id":"admission-init","result":{"protocolVersion":"openengine.cluster/v1","capabilities":{"graphProfiles":[],"logs":false},"status":{"phase":"empty","observedGeneration":null,"currentRunId":null,"atCursor":null}}} +{"jsonrpc":"2.0","id":"admission-init","result":{"protocolVersion":"openengine.cluster/v1","capabilities":{"graphProfiles":[],"logs":false,"agentAttach":false},"status":{"phase":"empty","observedGeneration":null,"currentRunId":null,"atCursor":null}}} {"id":"admission-plan","jsonrpc":"2.0","method":"plan","params":{"graph":{"initialInput":{"kind":"null"},"policy":{"default":"deny","policy":"policy.default@1"},"profile":"openengine.graph.single-worker/v1","root":{"attempts":1,"input":{"kind":"null"},"inputBindings":[],"kind":"step","name":"worker","output":{"kind":"null"},"timeoutMs":1000,"worker":"legacy.zeroshot.ship@1","writeBindings":[]}}}} {"jsonrpc":"2.0","id":"admission-plan","result":{"ok":true,"diagnostics":[],"bounds":{"termination":{"kind":"acyclic","order":["worker"]},"maxNodeExecutions":1,"peakConcurrency":1,"attemptsPerNode":{"worker":1}}}} {"id":"admission-apply","jsonrpc":"2.0","method":"apply","params":{"graph":{"initialInput":{"kind":"null"},"policy":{"default":"deny","policy":"policy.default@1"},"profile":"openengine.graph.single-worker/v1","root":{"attempts":1,"input":{"kind":"null"},"inputBindings":[],"kind":"step","name":"worker","output":{"kind":"null"},"timeoutMs":1000,"worker":"legacy.zeroshot.ship@1","writeBindings":[]}},"idempotencyKey":"golden-create","ifGeneration":0,"input":null}} diff --git a/protocol/openengine-cluster/v1/goldens/agent-attach-session.json b/protocol/openengine-cluster/v1/goldens/agent-attach-session.json new file mode 100644 index 00000000..c1b682de --- /dev/null +++ b/protocol/openengine-cluster/v1/goldens/agent-attach-session.json @@ -0,0 +1,28 @@ +[ + { + "event": { + "type": "working" + }, + "subscriptionId": "sub-1" + }, + { + "event": { + "text": "agent dispatch started", + "type": "output" + }, + "subscriptionId": "sub-1" + }, + { + "event": { + "text": "", + "type": "output" + }, + "subscriptionId": "sub-1" + }, + { + "event": { + "type": "settled" + }, + "subscriptionId": "sub-1" + } +] diff --git a/protocol/openengine-cluster/v1/goldens/initialize.ndjson b/protocol/openengine-cluster/v1/goldens/initialize.ndjson index 4c2c4828..4ee235ca 100644 --- a/protocol/openengine-cluster/v1/goldens/initialize.ndjson +++ b/protocol/openengine-cluster/v1/goldens/initialize.ndjson @@ -1,2 +1,2 @@ {"jsonrpc":"2.0","id":"init-1","method":"initialize","params":{"protocolVersion":"openengine.cluster/v1"}} -{"jsonrpc":"2.0","id":"init-1","result":{"protocolVersion":"openengine.cluster/v1","capabilities":{"graphProfiles":[],"logs":false},"status":{"phase":"empty","observedGeneration":null,"currentRunId":null,"atCursor":null}}} +{"jsonrpc":"2.0","id":"init-1","result":{"protocolVersion":"openengine.cluster/v1","capabilities":{"graphProfiles":[],"logs":false,"agentAttach":false},"status":{"phase":"empty","observedGeneration":null,"currentRunId":null,"atCursor":null}}} diff --git a/protocol/openengine-cluster/v1/openrpc.json b/protocol/openengine-cluster/v1/openrpc.json index d943f016..aaad8d0b 100644 --- a/protocol/openengine-cluster/v1/openrpc.json +++ b/protocol/openengine-cluster/v1/openrpc.json @@ -471,11 +471,33 @@ "$ref": "schema.json#/$defs/LogsResult" } } + }, + { + "name": "agent/attach", + "paramStructure": "by-name", + "params": [ + { + "name": "execution", + "required": true, + "schema": { + "maxLength": 128, + "minLength": 1, + "pattern": "^[^\\u0000-\\u001f\\u007f-\\u009f]+$", + "type": "string" + } + } + ], + "result": { + "name": "agentAttachResult", + "schema": { + "$ref": "schema.json#/$defs/AgentAttachResult" + } + } } ], "openrpc": "1.3.2", "x-generic-subscription-framing": { - "description": "watch and logs each establish a subscription via one normal JSON-RPC result; subsequent delivery uses the generic notification methods below, shared by every subscription-based method. There is no watch/event, watch/cancel, watch/closed, logs/event, logs/cancel, or logs/closed method on the wire.", + "description": "watch, logs, and agent/attach each establish a subscription via one normal JSON-RPC result; subsequent delivery uses the generic notification methods below, shared by every subscription-based method. There is no watch/event, watch/cancel, watch/closed, logs/event, logs/cancel, logs/closed, agent/attach/event, agent/attach/cancel, or agent/attach/closed method on the wire.", "notifications": { "event": { "$ref": "schema.json#/$defs/EventNotification" diff --git a/protocol/openengine-cluster/v1/schema.json b/protocol/openengine-cluster/v1/schema.json index b85dc4f0..c3581702 100644 --- a/protocol/openengine-cluster/v1/schema.json +++ b/protocol/openengine-cluster/v1/schema.json @@ -18,6 +18,121 @@ ], "type": "object" }, + "AgentAttachClosedNotification": { + "additionalProperties": false, + "description": "Wire body of the terminal `subscription/closed` server notification for an `agent/attach`\nsubscription. Deliberately carries no cursor field -- `agent/attach` gives a type-level\n\"cursorless\" guarantee, unlike [`crate::SubscriptionClosedNotification`].", + "properties": { + "reason": { + "$ref": "#/$defs/SubscriptionCloseReason" + }, + "subscriptionId": { + "type": "string" + } + }, + "required": [ + "subscriptionId", + "reason" + ], + "type": "object" + }, + "AgentAttachEvent": { + "description": "The closed public agent-attach progress algebra. This is the only representable shape:\nreasoning, tools, provider frames, usage, and session identifiers have no variant. `Working`\nand `Settled` are empty struct variants rather than bare units: serde's internally tagged enum\ndeserialization silently ignores unknown fields on a unit variant regardless of\n`deny_unknown_fields`, which would otherwise let an unrepresentable field ride along\nundetected on either of these two variants.", + "oneOf": [ + { + "additionalProperties": false, + "properties": { + "type": { + "const": "working", + "type": "string" + } + }, + "required": [ + "type" + ], + "type": "object" + }, + { + "additionalProperties": false, + "properties": { + "text": { + "maxLength": 16384, + "pattern": "^[^\\u0000-\\u001f\\u007f-\\u009f]*$", + "type": "string" + }, + "type": { + "const": "output", + "type": "string" + } + }, + "required": [ + "type", + "text" + ], + "type": "object" + }, + { + "additionalProperties": false, + "properties": { + "type": { + "const": "settled", + "type": "string" + } + }, + "required": [ + "type" + ], + "type": "object" + } + ] + }, + "AgentAttachEventNotification": { + "$ref": "#/$defs/AgentAttachEventNotificationWire" + }, + "AgentAttachEventNotificationWire": { + "additionalProperties": false, + "properties": { + "event": { + "$ref": "#/$defs/AgentAttachEvent" + }, + "subscriptionId": { + "type": "string" + } + }, + "required": [ + "subscriptionId", + "event" + ], + "type": "object" + }, + "AgentAttachParams": { + "additionalProperties": false, + "description": "`agent/attach` establishment parameters: the named `{execution}` request. Deliberately closed,\nrejecting any unknown field.", + "properties": { + "execution": { + "maxLength": 128, + "minLength": 1, + "pattern": "^[^\\u0000-\\u001f\\u007f-\\u009f]+$", + "type": "string" + } + }, + "required": [ + "execution" + ], + "type": "object" + }, + "AgentAttachResult": { + "additionalProperties": false, + "description": "The `agent/attach` establishment result: only a `subscriptionId`. Deliberately carries no\n`runId` or `atCursor` -- `agent/attach` is not run-scoped and has no cursor.", + "properties": { + "subscriptionId": { + "type": "string" + } + }, + "required": [ + "subscriptionId" + ], + "type": "object" + }, "ApplyParams": { "additionalProperties": false, "properties": { @@ -1690,6 +1805,44 @@ ], "type": "object" }, + "JsonRpcNotification6": { + "properties": { + "jsonrpc": { + "type": "string" + }, + "method": { + "type": "string" + }, + "params": { + "$ref": "#/$defs/AgentAttachEventNotification" + } + }, + "required": [ + "jsonrpc", + "method", + "params" + ], + "type": "object" + }, + "JsonRpcNotification7": { + "properties": { + "jsonrpc": { + "type": "string" + }, + "method": { + "type": "string" + }, + "params": { + "$ref": "#/$defs/AgentAttachClosedNotification" + } + }, + "required": [ + "jsonrpc", + "method", + "params" + ], + "type": "object" + }, "JsonRpcRequest": { "properties": { "id": { @@ -1759,6 +1912,29 @@ ], "type": "object" }, + "JsonRpcRequest12": { + "properties": { + "id": { + "$ref": "#/$defs/RequestId" + }, + "jsonrpc": { + "type": "string" + }, + "method": { + "type": "string" + }, + "params": { + "$ref": "#/$defs/AgentAttachParams" + } + }, + "required": [ + "jsonrpc", + "id", + "method", + "params" + ], + "type": "object" + }, "JsonRpcRequest2": { "properties": { "id": { @@ -1973,6 +2149,16 @@ } ] }, + "JsonRpcResponse12": { + "anyOf": [ + { + "$ref": "#/$defs/JsonRpcSuccess12" + }, + { + "$ref": "#/$defs/JsonRpcErrorResponse" + } + ] + }, "JsonRpcResponse2": { "anyOf": [ { @@ -2110,6 +2296,25 @@ ], "type": "object" }, + "JsonRpcSuccess12": { + "properties": { + "id": { + "$ref": "#/$defs/RequestId" + }, + "jsonrpc": { + "type": "string" + }, + "result": { + "$ref": "#/$defs/AgentAttachResult" + } + }, + "required": [ + "jsonrpc", + "id", + "result" + ], + "type": "object" + }, "JsonRpcSuccess2": { "properties": { "id": { @@ -2902,6 +3107,10 @@ "ServerCapabilities": { "additionalProperties": false, "properties": { + "agentAttach": { + "default": false, + "type": "boolean" + }, "graphProfiles": { "default": [], "items": { @@ -3580,6 +3789,18 @@ }, "$schema": "https://json-schema.org/draft/2020-12/schema", "properties": { + "agent_attach_closed_notification": { + "$ref": "#/$defs/JsonRpcNotification7" + }, + "agent_attach_event_notification": { + "$ref": "#/$defs/JsonRpcNotification6" + }, + "agent_attach_request": { + "$ref": "#/$defs/JsonRpcRequest12" + }, + "agent_attach_response": { + "$ref": "#/$defs/JsonRpcResponse12" + }, "apply_request": { "$ref": "#/$defs/JsonRpcRequest3" }, @@ -3689,7 +3910,11 @@ "logs_request", "logs_response", "log_event_notification", - "logs_closed_notification" + "logs_closed_notification", + "agent_attach_request", + "agent_attach_response", + "agent_attach_event_notification", + "agent_attach_closed_notification" ], "title": "ImplementedProtocolSchema", "type": "object" diff --git a/zeroshot-rust/tests/backend_boundary.rs b/zeroshot-rust/tests/backend_boundary.rs index c469ec7a..ebc3e32e 100644 --- a/zeroshot-rust/tests/backend_boundary.rs +++ b/zeroshot-rust/tests/backend_boundary.rs @@ -55,13 +55,13 @@ async fn production_dispatcher_returns_canonical_empty_initialize_and_get() { initialize["result"], json!({ "protocolVersion": PROTOCOL_VERSION, - "capabilities": { "graphProfiles": [], "logs": false }, + "capabilities": { "graphProfiles": [], "logs": false, "agentAttach": false }, "status": empty_status() }) ); assert_eq!( initialize["result"]["capabilities"], - json!({ "graphProfiles": [], "logs": false }) + json!({ "graphProfiles": [], "logs": false, "agentAttach": false }) ); let get = dispatch("get", json!({"atCursor": null})).await;