Skip to content

test: cover newly-stabilized logout, MCP-over-ACP, and Error edge cases - #7

Draft
cursor[bot] wants to merge 3 commits into
mainfrom
cursor/missing-test-coverage-3f10
Draft

test: cover newly-stabilized logout, MCP-over-ACP, and Error edge cases#7
cursor[bot] wants to merge 3 commits into
mainfrom
cursor/missing-test-coverage-3f10

Conversation

@cursor

@cursor cursor Bot commented May 31, 2026

Copy link
Copy Markdown

Risky behavior now covered

Three areas that recently changed or had thin coverage and would silently regress on the wire if anything shifted:

  • Logout method (just stabilized in feat: Stabilize logout method agentclientprotocol/agent-client-protocol#1273) had zero unit tests after losing its #[cfg(feature = "unstable_logout")] gates. Wire-format pinning, _meta round-tripping, AGENT_METHOD_NAMES.logout/ClientRequest::method() linkage, default-on-error tolerance on AgentAuthCapabilities::logout, and threading through AgentCapabilities.auth.
  • MCP-over-ACP (recent feature feat(unstable): Add experimental MCP-over-ACP message types agentclientprotocol/agent-client-protocol#1185) only had one method-name smoke test in agent.rs. The actual mcp.rs types (ConnectMcp{Request,Response}, MessageMcp{Request,Notification,Response}, DisconnectMcp{Request,Response}) had no serialization tests. Particularly important: the docs-stated invariant that omitted params and explicit null params MUST both decode to None, and that MessageMcpResponse (a transparent Arc<RawValue> wrapper) forwards inner MCP results verbatim.
  • Error had only ErrorCode numeric round-trip coverage. The Display impl (empty-message fallback, pretty-printed data suffix), From<serde_json::Error> (InvalidParams + message in data), From<anyhow::Error> downcast behavior (the bug-prone branch that must preserve an inner ACP Error instead of re-wrapping it as InternalError), into_internal_error, and resource_not_found(uri) with/without URI were all untested.

Test files added/updated

  • src/v1/error.rs — 11 new tests for Error::Display, conversions, constructors, and resource_not_found URI attachment.
  • src/v1/agent.rs — 7 new tests for the stabilized logout method, LogoutRequest/Response/Capabilities, and AgentAuthCapabilities (including the malformed-field tolerance behavior baked in by DefaultOnError).
  • src/v1/mcp.rs — new tests module with 11 tests pinning the MCP-over-ACP wire format end-to-end.
  • src/v2/{error,agent,mcp}.rs — mirrored tests in the v2 scaffold so the parallel module can't drift silently.

Net: +60 tests (268 → 328 with --all-features; 41 → 60 default features). cargo test --lib, cargo test --lib --all-features, cargo clippy --all-features, and cargo fmt --check all pass.

Why these tests materially reduce regression risk

  • Logout is freshly stabilized. Stabilization PRs that move code out from under #[cfg] flags often silently lose test coverage; pinning the wire name ("logout"), the {} request/response shape, the _meta round-trip, the "supplying {} means supported" capability convention, and the DefaultOnError tolerance closes that gap before any client integrates against it.
  • MCP-over-ACP is brand new and feature-flagged. Any change to field naming (camelCase via serde), omission semantics (skip_serializing_none + Option<Map>), or method-name constants (mcp/connect, mcp/message, mcp/disconnect) would silently break every IDE/agent pair. The "omitted vs explicit null params" test specifically guards a documented invariant that's easy to lose during refactors.
  • Error conversions are infrastructure for every method. The anyhow::Error downcast in particular is the kind of subtle code that's easy to "fix" into re-wrapping behavior; the new test makes that an immediate failure. The Display impl handles two formatting branches (empty message, data suffix) that could regress to a bare numeric string or panic on data formatting.

All tests are deterministic, self-contained (serde_json only, no I/O, no concurrency), and run in the existing cargo test --lib pipeline.

Open in Web View Automation 

Summary by cubic

Add tests to pin the wire format and behavior for the stabilized logout method, MCP-over-ACP connect/message/disconnect, and Error conversions/formatting across v1 and v2. Tests only; no runtime changes.

  • New Tests
    • Logout: method name "logout", {} request/response, _meta round-trip, capability advertising via {}, and DefaultOnError tolerance in AgentAuthCapabilities/AgentCapabilities.
    • MCP-over-ACP: method constants ("mcp/connect", "mcp/message", "mcp/disconnect"); ConnectMcp*/MessageMcp*/DisconnectMcp* serialization; omitted vs null params both decode to None; MessageMcpResponse forwards inner JSON verbatim; _meta round-trip.
    • Error: Display (empty message falls back to code; pretty data suffix); From<serde_json::Error>InvalidParams with message in data; From<anyhow::Error> preserves inner ACP Error else InternalError; into_internal_error; resource_not_found with/without URI; numeric code and JSON-RPC object shape pinned.

Written for commit 6adb0fe. Summary will update on new commits.

Review in cubic

Note

Add tests covering logout, MCP-over-ACP, and Error wire behavior for v1 and v2

  • Adds serialization round-trip tests for logout (LogoutRequest/LogoutResponse) and AgentAuthCapabilities in both src/v1/agent.rs and src/v2/agent.rs, including DefaultOnError tolerance for malformed logout values.
  • Pins MCP-over-ACP wire surface in src/v1/mcp.rs and src/v2/mcp.rs: method name constants, connect/disconnect round-trips, message params omission when unset, null decoding, and verbatim RawValue preservation.
  • Covers ErrorCode and Error behaviors in src/v1/error.rs and src/v2/error.rs: numeric conversions, constructor codes, Display output, From<serde_json::Error> and From<anyhow::Error> mappings, and JSON-RPC object serialization shape.

Macroscope summarized 6adb0fe.

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