Skip to content

feat: CEP-41 - open-stream transport wiring, call_tool_stream, e2e#98

Merged
ContextVM-org merged 2 commits into
ContextVM:mainfrom
harsh04044:feat/cep41-pr2-transport-wiring
Jun 24, 2026
Merged

feat: CEP-41 - open-stream transport wiring, call_tool_stream, e2e#98
ContextVM-org merged 2 commits into
ContextVM:mainfrom
harsh04044:feat/cep41-pr2-transport-wiring

Conversation

@harsh04044

@harsh04044 harsh04044 commented Jun 24, 2026

Copy link
Copy Markdown

Second and final PR for CEP-41. Wires the PR1 engine into both transports and makes open-ended streaming live end-to-end.

What's in this PR:

Capability advertisement + learning - client advertises support_open_stream when enabled, OR-learns server support. Server advertises via announcements and first responses, OR-learns from inbound client tags. Disabled by default (opt-in via with_open_stream(OpenStreamConfig::enabled())), matching the TS SDK.

Server wiring - writer created on tools/call with a progressToken, captures a RouteSnapshot (client_pubkey, request_id, wrap_kind) at creation so the deferred final response can be sent even after the route store sweeps the entry. Response deferral sits at the top of send_response: a started writer stashes the response and returns immediately, an unstarted writer drops and sends normally. Inbound interception routes start/ping/pong/abort/close to the right handler (writer or server-as-reader). Keepalive sweep drives tick(now) on all sessions.

Client wiring - inbound interception feeds the reader session, forwards stripped+token-restored progress for rmcp's reset_timeout_on_progress, and touches the correlation entry to keep it alive. A bind_lock serializes the push→bind window so concurrent call_tool_stream calls don't cross tokens.

call_tool_stream - free function returning ToolStreamCall { progress_token, stream, result, abort }. Builds its own PeerRequestOptions with no hard lifetime cap unless max_total_timeout_ms is explicitly set.

rmcp worker injection - OpenStreamWriter is injected into request extensions after conversion, before dispatch, so tool handlers reach it via ctx.extensions.

Also fixes a bug: the oversized reassembly path dispatched tools/call directly, bypassing writer creation, so an oversized request with a progressToken never got a writer and never streamed. Fixed by creating the writer on the reassembled end frame.

10 e2e tests covering roundtrip (string + numeric token), deferred response after close, client abort, gate off, unstarted writer, concurrent stream isolation, CEP-22 + CEP-41 composition (oversized request + streaming response), oversized response alongside a separate live stream, and plain call with progressToken not interfering with a live stream. 737 tests passing.

After review: FFI mirror updated (supports_open_stream now visible to Python/Swift/Kotlin consumers), default reverted to false (TS parity), ToolStreamCall::result flattened to BoxFuture, bind_lock deadlock fixed with tokio::select!, e-tag correlated_event_id cleaned up to Option<&str>.

Closes #96

@ContextVM-org

ContextVM-org commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

The core streaming path (server→client via call_tool_stream) is correct, well-tested, and faithful to the TS reference. Deferral ordering, the oversized-reassembly bug fix, the deterministic keepalive clock, and concurrency isolation all check out. Full quality gate is green: 737 tests pass, clippy/fmt/doc/no-default-features clean. Approving with the follow-ups below.

Should fix before merge

1. CHANGELOG.md not updated. This PR adds major public API (call_tool_stream, ToolStreamCall, ClientOpenStreamHandle, default-flipped OpenStreamConfig) but the changelog still ends at [0.1.1]. AGENTS.md requires an entry for user-visible changes.

2. FFI PeerCapabilities mirror is missing supports_open_stream. src/transport/discovery_tags.rs has the field, but the UniFFI mirror (contextvm-ffi/src/uniffi_types.rs:95) and capabilities_to_uniffi (:358) don't. Python/Swift/Kotlin discovery consumers silently drop the open-stream capability — exactly the drift AGENTS.md warns about. (Introduced in PR1, but this PR finalizes the feature and should close the loop.)

Worth correcting in the PR body

3. "Enabled by default … matching the TS SDK" is inaccurate. The TS SDK defaults open-stream to false on both sides (nostr-client-transport.ts:214, nostr-server-transport.ts:319). The Rust SDK defaults to true. Defaulting to true is defensible and safe, but it's a deliberate behavioral divergence from the TS peer, not parity — worth stating correctly so it's an intentional choice.

Non-blocking follow-ups (file or defer)

4. ToolStreamCall::result is a raw JoinHandle → consumers get Result<Result<CallToolResult, ServiceError>, JoinError>. Works, but it's a doubly-nested result. Consider a BoxFuture wrapper or at least documenting the shape.

5. call_tool_stream holds the bind_lock across an unbounded pending.await. If rmcp rejects before publishing, the placeholder is never bound and every subsequent call_tool_stream deadlocks. Mirrors the TS pattern (same risk there), so not a regression — but a defensive resolution would be more robust.

6. Server-as-reader keepalive ping has no e-tag correlationevent_id_for_token returns None for inbound (client→server) streams since token_to_event is only populated for server→client writers. Benign today (no client API to initiate client→server streams, handled gracefully), but latent if bidirectional streaming is ever exposed.

Nothing here blocks the core feature. 1 and 2 are quick; 3 is a doc/wording fix. Happy to see this land.

@harsh04044

Copy link
Copy Markdown
Author

addressed all 5 review points - FFI mirror fixed (Python/Swift/Kotlin consumers now see supports_open_stream), default reverted to false (genuine TS parity), result flattened to BoxFuture so consumers await once, bind_lock deadlock fixed with tokio::select! + cancel_outbound, and the e-tag correlated_event_id cleaned up to Option<&str> with a comment marking the bidirectional streaming breadcrumb. 737 tests passing, all gates green including --no-default-features.

changelog will be updated in this PR #95 as a new commit

@ContextVM-org ContextVM-org merged commit ea536b5 into ContextVM:main Jun 24, 2026
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

implement CEP-41 open-stream

2 participants