Skip to content

fix(acp)!: tighten the root connection and builder surface for multi-version use - #477

Merged
benbrandt merged 11 commits into
mainfrom
cleanup-core-connection
Nov 1, 2026
Merged

benbrandt merged 11 commits into
mainfrom
cleanup-core-connection

Conversation

@benbrandt

@benbrandt benbrandt commented Oct 31, 2026 •

Copy link
Copy Markdown
Member

Part of the 5.0 public-API cleanup. Tightens the version-neutral root connection and builder surface so that adding later protocol versions stays additive.

Addressed

  • F01 Remove Builder::with_v2_protocol_guard (all four impls). The 2.1.0 changelog entry is left as history; the migration guide records the removal.
  • F15 Merge ConnectionTo::spawn_connection_with_context into spawn_connection, which now returns P::Connection<R::Counterpart> (a raw builder still returns the raw ConnectionTo).
  • F18 ConnectionTo::into_protocol::<P>() returns Result and fails with an Internal error (-32603) when the builder's guard selected another version. The mismatch is a local programming error, so it is not reported to the peer as Invalid Request. Adds ConnectionTo::protocol_version().
  • F04 Typed send_cancel_request(_to) take impl Into<schema::v1::RequestId> / impl Into<schema::v2::RequestId>. v1 stays source-compatible because RawJsonRpcId converts into schema::v1::RequestId. Only v2 callers passing a RawJsonRpcId must switch.
  • G82 From<v1::ConnectionTo<C>> and From<v2::ConnectionTo<C>> for ConnectionTo<C>, and Protocol::Connection requires Into<ConnectionTo<C>>.
  • F20 InitializationVersionMismatch and SelectedInitializeResponse move to the crate root (still behind unstable_protocol_v2). InitializeOutcome stays in v2::.
  • F75 Drop the redundant InitializationVersionMismatch::selected field. Use the selected() method instead.
  • F27 ProxyProtocolRouter shares AgentProtocolRouter's answer-and-continue selection loop. It no longer closes the connection on the first rejected or malformed frame. Adds a retry test and a test for a v1+v2 router that answers invalid JSON and a batch with id: null errors, then forwards the next valid proxy/initialize to the selected implementation unchanged. The rejection-drain tests in protocol_driver_finish.rs are replaced by one that checks the rejection is flushed over Lines while input stays open and that the router finishes on input EOF.
  • F38 Channel is #[non_exhaustive], with Channel::new and Channel::into_parts. The rustdoc now states that the unbounded mpsc pair is the contract.
  • F39 JsonRpcMessage::parse_message takes params: &serde_json::Value. This changes the signature only, and the derive macro output is updated to match.
  • F50 SentRequest and PreparedRequest gain into_raw and, on Raw, into_protocol.

Skipped

  • F69 SelectedInitializeRequest is defined in the conductor crate, not in core. It belongs to the conductor cleanup.

Notes

  • The http crate and core tests needed small edits: .. in Channel patterns and the new parse_message callers.
  • The derive crate output changed for F39, so core and derive have to be released together.
  • session.rs's ensure_v1_session_protocol is removed (24368a4). A v1::ConnectionTo now only comes from a .v1() builder or from into_protocol::<V1>(), which fails on a v2-guarded connection, so the check could no longer fire. fix(acp)!: v1 session stream reports prompt errors and full responses #474 also edits those call sites, so whichever of the two merges second needs a small rebase.
  • With F27, a proxy router that has v1 configured now answers a malformed frame (invalid JSON or a batch) before initialization with an error, as AgentProtocolRouter does. It used to hand that frame to the v1 implementation.

Checks

Rerun at 3791556:

  • cargo fmt -- --check: pass
  • cargo clippy --workspace --all-targets --all-features -- -D warnings and cargo clippy -- -D warnings: pass
  • cargo hack check --workspace --feature-powerset --depth 2 --exclude-features default,unstable,wasm_js --no-dev-deps: pass, no warnings
  • cargo test --workspace --all-features --locked --no-fail-fast: pass, except agent-client-protocol-derive --lib, which fails to load libstd under the local test runner (it does on main too)
  • cargo test -p agent-client-protocol --no-default-features --locked: pass
  • RUSTDOCFLAGS="-D warnings" cargo doc --all-features --no-deps for core, derive, http and test crates: pass
  • mdbook build: pass

Rename JsonRpcResponse::from_value → from_json (78d28e5)

JsonRpcResponse paired into_json with from_value. The parser is now from_json, so the pair reads consistently. Renaming a trait method after 5.0 would be breaking. Hand-written impls and direct calls change; derived impls change with the derive macros. RawJsonRpcParams::from_value is an unrelated inherent method and keeps its name. md/migration-v5.md documents it.

Rebased onto main after #474, #476 and #478 merged. Resolved conflicts in md/migration-v5.md, and dropped this branch's edit to util::TypeNotification, which #478 removed.

benbrandt and others added 11 commits November 1, 2026 21:18
Remove `Builder::with_v2_protocol_guard`, a v2-named method on the
version-neutral builder with no v1 or proxy counterpart and no callers.

Merge `ConnectionTo::spawn_connection_with_context` into
`spawn_connection`, which now returns the connection type the child
builder's callbacks receive, as the v1 and v2 connections already did.

`ConnectionTo::into_protocol::<P>()` now returns a `Result` and fails
when the builder's guard selected a different version, so a v1
connection can no longer be retyped to send v2 payloads. Add
`ConnectionTo::protocol_version()`.

The typed `send_cancel_request(_to)` on `v1::ConnectionTo` and
`v2::ConnectionTo` take the version's own request ID, so v2 can no
longer send `{"requestId": null}`.

`v1::ConnectionTo` and `v2::ConnectionTo` convert into the root
`ConnectionTo` with `From`, and `Protocol::Connection` requires it, so
`P`-generic code can reach the raw connection.

Findings F01, F15, F18, F04, G82.

BREAKING CHANGE: `Builder::with_v2_protocol_guard` and
`ConnectionTo::spawn_connection_with_context` are removed;
`ConnectionTo::spawn_connection` returns `P::Connection` and takes two
type parameters; `ConnectionTo::into_protocol` returns a `Result`; the
typed `send_cancel_request(_to)` methods take typed request IDs.

Co-Authored-By: Claude <noreply@anthropic.com>
`InitializationVersionMismatch` and `SelectedInitializeResponse` are not
tied to protocol v2: every later version's negotiation reports a lower
selection with them. They now live at the crate root (still behind
`unstable_protocol_v2`, the first feature under which a lower version
can be selected). `v2::InitializeOutcome` stays in `v2`.

Drop the redundant `InitializationVersionMismatch::selected` field in
favor of a `selected()` method that reads the response's version, so the
two can no longer disagree.

Findings F20, F75.

BREAKING CHANGE: `v2::InitializationVersionMismatch` and
`v2::SelectedInitializeResponse` move to the crate root, and
`InitializationVersionMismatch::selected` is now a method.

Co-Authored-By: Claude <noreply@anthropic.com>
…lize

`ProxyProtocolRouter` closed the connection on the first rejected
`proxy/initialize`, unsupported version, or malformed frame, while its
rustdoc said it selected "exactly as `Agent::protocol_router` does".
Both routers now share one selection loop that answers each rejected
frame and keeps reading, so a conductor can retry `proxy/initialize`
the same way a client can retry `initialize`.

Finding F27.

Co-Authored-By: Claude <noreply@anthropic.com>
- `Channel` is `#[non_exhaustive]` with `Channel::new` and
  `Channel::into_parts`, and documents that the unbounded mpsc pair is
  the intended contract, so fields can be added later.
- `JsonRpcMessage::parse_message` takes `params: &serde_json::Value`
  instead of `&impl Serialize`. Every SDK caller already passes a JSON
  value, and a concrete type lets implementations avoid a serialize
  round trip later without another break. The derive macros
  generate the new signature.
- `SentRequest` and `PreparedRequest` gain `into_raw` and, on `Raw`
  handles, `into_protocol`, like `Responder` and `ResponseRouter`.

Findings F38, F39, F50.

BREAKING CHANGE: `Channel` can no longer be built or destructured
exhaustively outside the crate; hand-written `JsonRpcMessage`
implementations must change the `parse_message` signature.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Removing with_v2_protocol_guard left the import unused when only
unstable_protocol_v2 is enabled.

Co-Authored-By: Claude <noreply@anthropic.com>
…tics

ProxyProtocolRouter no longer finishes after a rejected proxy/initialize,
so the rejection-drain tests no longer applied. Check instead that the
rejection reaches the wire while input stays open and that the router
finishes on input EOF.

Co-Authored-By: Claude <noreply@anthropic.com>
A version mismatch in `ConnectionTo::into_protocol` is a local programming
error. Reporting it as Invalid Request (-32600) told the remote peer its
request was malformed when a handler propagated it with `?`. It is now an
Internal error (-32603).

Also tidy the migration and protocol-v2 notes: the v1 `send_cancel_request`
change is source-compatible for `RawJsonRpcId` arguments, and the crate-root
initialize mismatch types are described without claiming every version uses
them.

Co-Authored-By: Claude <noreply@anthropic.com>
…router

Co-Authored-By: Claude <noreply@anthropic.com>
ensure_v1_session_protocol rejected v1 session builders on a v2-guarded
connection. A v1::ConnectionTo now only comes from a .v1() builder or from
ConnectionTo::into_protocol::<V1>(), which fails for a v2-guarded
connection, so the check can no longer fire.

Co-Authored-By: Claude <noreply@anthropic.com>
JsonRpcResponse paired into_json with from_value. Rename the parser to
from_json so the two read as a pair, before 5.0 makes the trait's method
names hard to change.

BREAKING CHANGE: JsonRpcResponse::from_value is renamed to from_json.
Hand-written implementations and direct calls must use the new name;
derived implementations change with the derive macros.

Co-Authored-By: Claude <noreply@anthropic.com>
@benbrandt
benbrandt force-pushed the cleanup-core-connection branch from 24368a4 to 78d28e5 Compare November 1, 2026 20:27
@benbrandt
benbrandt enabled auto-merge (squash) November 1, 2026 20:33
@benbrandt
benbrandt merged commit 79a3501 into main Nov 1, 2026
13 checks passed
@benbrandt
benbrandt deleted the cleanup-core-connection branch November 1, 2026 20:33
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.

1 participant