test: add regression coverage for logout, provider methods, and tool-call merges

AGENTS.md

@@ -47,14 +47,14 @@ This is a **protocol schema library** (not a runtime application). There are no
 
 ### Key commands (see `package.json` scripts)
 
-| Command | Purpose |
-|---|---|
-| `npm run check` | Full CI pipeline: clippy, format check, spellcheck, tests |
-| `npm run generate` | Regenerate JSON schemas from Rust types + format |
-| `cargo test --all-features` | Run Rust unit + doc tests |
-| `cargo clippy --all-features` | Lint Rust code |
-| `npm run format:check` | Verify Prettier + rustfmt formatting |
-| `npm run format` | Auto-fix formatting |
+| Command                       | Purpose                                                   |
+| ----------------------------- | --------------------------------------------------------- |
+| `npm run check`               | Full CI pipeline: clippy, format check, spellcheck, tests |
+| `npm run generate`            | Regenerate JSON schemas from Rust types + format          |
+| `cargo test --all-features`   | Run Rust unit + doc tests                                 |
+| `cargo clippy --all-features` | Lint Rust code                                            |
+| `npm run format:check`        | Verify Prettier + rustfmt formatting                      |
+| `npm run format`              | Auto-fix formatting                                       |
 
 ### Gotchas
 

src/v1/agent.rs

@@ -6467,4 +6467,163 @@ mod test_serialization {
         let deserialized: AgentCapabilities = serde_json::from_value(json).unwrap();
         assert!(deserialized.providers.is_some());
     }
+
+    // --- Logout method stabilization coverage (PR #1273) ---
+    //
+    // The logout method was promoted from `unstable_logout` to the stable API in
+    // PR #1273 without any accompanying tests. These tests lock in the wire
+    // format and enum routing so future refactors can't silently break clients.
+
+    #[test]
+    fn test_logout_method_name_constant() {
+        assert_eq!(AGENT_METHOD_NAMES.logout, "logout");
+        assert_eq!(LOGOUT_METHOD_NAME, "logout");
+    }
+
+    #[test]
+    fn test_logout_client_request_method_dispatch() {
+        assert_eq!(
+            ClientRequest::LogoutRequest(LogoutRequest::new()).method(),
+            "logout"
+        );
+    }
+
+    #[test]
+    fn test_logout_request_serializes_to_empty_object() {
+        let request = LogoutRequest::new();
+        let json = serde_json::to_value(&request).unwrap();
+        assert_eq!(json, json!({}));
+
+        let round_trip: LogoutRequest = serde_json::from_value(json).unwrap();
+        assert_eq!(round_trip, request);
+    }
+
+    #[test]
+    fn test_logout_response_serializes_to_empty_object() {
+        let response = LogoutResponse::new();
+        let json = serde_json::to_value(&response).unwrap();
+        assert_eq!(json, json!({}));
+
+        let round_trip: LogoutResponse = serde_json::from_value(json).unwrap();
+        assert_eq!(round_trip, response);
+    }
+
+    #[test]
+    fn test_logout_request_accepts_missing_params() {
+        // `session/prompt`-style handlers may omit `params` entirely for
+        // parameter-less methods. Verify LogoutRequest tolerates that shape
+        // when decoded from an empty JSON object.
+        let request: LogoutRequest = serde_json::from_str("{}").unwrap();
+        assert!(request.meta.is_none());
+    }
+
+    #[test]
+    fn test_logout_response_default_deserialization() {
+        // The AgentResponse enum wraps LogoutResponse with `#[serde(default)]`,
+        // which relies on LogoutResponse: Default. This documents that contract.
+        let response: LogoutResponse = LogoutResponse::default();
+        assert_eq!(response, LogoutResponse::new());
+    }
+
+    #[test]
+    fn test_agent_auth_capabilities_default_is_empty() {
+        let caps = AgentAuthCapabilities::default();
+        let json = serde_json::to_value(&caps).unwrap();
+        // No logout capability advertised -> emit an empty object so peers can
+        // detect the absence of features without inspecting each optional key.
+        assert_eq!(json, json!({}));
+        assert!(caps.logout.is_none());
+    }
+
+    #[test]
+    fn test_agent_auth_capabilities_with_logout() {
+        let caps = AgentAuthCapabilities::new().logout(LogoutCapabilities::new());
+        let json = serde_json::to_value(&caps).unwrap();
+        assert_eq!(
+            json,
+            json!({
+                "logout": {}
+            })
+        );
+
+        let deserialized: AgentAuthCapabilities = serde_json::from_value(json).unwrap();
+        assert!(deserialized.logout.is_some());
+    }
+
+    #[test]
+    fn test_agent_auth_capabilities_tolerates_invalid_logout() {
+        // `logout` uses `#[serde_as(deserialize_as = "DefaultOnError")]`, so a
+        // peer that sends a malformed value should not fail initialization.
+        // This protects backwards compatibility if the shape ever evolves.
+        let json = json!({
+            "logout": "not-an-object"
+        });
+        let caps: AgentAuthCapabilities = serde_json::from_value(json).unwrap();
+        assert!(caps.logout.is_none());
+    }
+
+    #[test]
+    fn test_agent_capabilities_missing_auth_defaults_to_empty() {
+        // Existing clients on the older schema don't send an `auth` field.
+        // The `#[serde(default)]` on `AgentCapabilities.auth` guarantees a
+        // silent, non-breaking upgrade path now that logout is stable.
+        let json = json!({});
+        let caps: AgentCapabilities = serde_json::from_value(json).unwrap();
+        assert_eq!(caps.auth, AgentAuthCapabilities::default());
+        assert!(caps.auth.logout.is_none());
+    }
+
+    #[test]
+    fn test_agent_capabilities_with_auth_logout_serializes() {
+        let caps = AgentCapabilities::new()
+            .auth(AgentAuthCapabilities::new().logout(LogoutCapabilities::new()));
+
+        let json = serde_json::to_value(&caps).unwrap();
+        assert_eq!(json["auth"], json!({ "logout": {} }));
+
+        let deserialized: AgentCapabilities = serde_json::from_value(json).unwrap();
+        assert!(deserialized.auth.logout.is_some());
+    }
+
+    // --- Provider method rename coverage (PR #1272) ---
+    //
+    // PR #1272 renamed the provider method types from plural to singular
+    // (`providers/set` / `providers/disable`) since both operate on a single
+    // provider. Without direct wire-format tests, a future rename could
+    // silently drift the constant back to the plural form and break clients.
+
+    #[cfg(feature = "unstable_llm_providers")]
+    #[test]
+    fn test_providers_method_name_constants_are_correct() {
+        assert_eq!(AGENT_METHOD_NAMES.providers_list, "providers/list");
+        assert_eq!(AGENT_METHOD_NAMES.providers_set, "providers/set");
+        assert_eq!(AGENT_METHOD_NAMES.providers_disable, "providers/disable");
+
+        assert_eq!(PROVIDERS_LIST_METHOD_NAME, "providers/list");
+        assert_eq!(PROVIDERS_SET_METHOD_NAME, "providers/set");
+        assert_eq!(PROVIDERS_DISABLE_METHOD_NAME, "providers/disable");
+    }
+
+    #[cfg(feature = "unstable_llm_providers")]
+    #[test]
+    fn test_client_request_provider_method_dispatch() {
+        assert_eq!(
+            ClientRequest::ListProvidersRequest(ListProvidersRequest::new()).method(),
+            "providers/list"
+        );
+        assert_eq!(
+            ClientRequest::SetProviderRequest(SetProviderRequest::new(
+                "main",
+                LlmProtocol::Anthropic,
+                "https://api.anthropic.com",
+            ))
+            .method(),
+            "providers/set"
+        );
+        assert_eq!(
+            ClientRequest::DisableProviderRequest(DisableProviderRequest::new("secondary"))
+                .method(),
+            "providers/disable"
+        );
+    }
 }

src/v1/tool_call.rs

@@ -672,3 +672,206 @@ impl ToolCallLocation {
         self
     }
 }
+
+#[cfg(test)]
+mod tests {
+    //! Coverage for tool-call merge semantics and wire format.
+    //!
+    //! These types drive one of the most visible surfaces of the protocol
+    //! (tool execution progress in the client UI) and previously had no
+    //! dedicated tests. In particular:
+    //!
+    //! * [`ToolCall::update`] applies partial updates in place and is the
+    //!   canonical way agents report progress. A mistake here would silently
+    //!   drop or overwrite user-visible tool state.
+    //! * `TryFrom<ToolCallUpdate> for ToolCall` and `From<ToolCall> for
+    //!   ToolCallUpdate` are the bridge between "first update" and "existing
+    //!   tool call" flows and depend on a `title` invariant that is easy to
+    //!   regress.
+    //! * The `#[serde(skip_serializing_if)]` guards on the default
+    //!   [`ToolKind`] and [`ToolCallStatus`] variants keep the wire format
+    //!   compact and backwards-compatible with older clients.
+
+    use super::*;
+    use crate::ErrorCode;
+    use serde_json::json;
+
+    fn sample_tool_call() -> ToolCall {
+        ToolCall::new("call-1", "Read file")
+            .kind(ToolKind::Read)
+            .status(ToolCallStatus::InProgress)
+            .content(vec![ToolCallContent::from("partial")])
+            .locations(vec![ToolCallLocation::new("/tmp/a.txt").line(3u32)])
+            .raw_input(json!({ "path": "/tmp/a.txt" }))
+    }
+
+    #[test]
+    fn tool_call_update_applies_all_provided_fields() {
+        let mut call = sample_tool_call();
+        let fields = ToolCallUpdateFields::new()
+            .title("Reading file")
+            .kind(ToolKind::Execute)
+            .status(ToolCallStatus::Completed)
+            .content(vec![ToolCallContent::from("done")])
+            .locations(vec![ToolCallLocation::new("/tmp/b.txt")])
+            .raw_input(json!({ "path": "/tmp/b.txt" }))
+            .raw_output(json!({ "bytes": 42 }));
+
+        call.update(fields);
+
+        assert_eq!(call.title, "Reading file");
+        assert_eq!(call.kind, ToolKind::Execute);
+        assert_eq!(call.status, ToolCallStatus::Completed);
+        assert_eq!(call.content.len(), 1);
+        assert_eq!(call.locations.len(), 1);
+        assert_eq!(call.locations[0].path, PathBuf::from("/tmp/b.txt"));
+        assert_eq!(call.raw_input, Some(json!({ "path": "/tmp/b.txt" })));
+        assert_eq!(call.raw_output, Some(json!({ "bytes": 42 })));
+    }
+
+    #[test]
+    fn tool_call_update_with_empty_fields_leaves_state_unchanged() {
+        let mut call = sample_tool_call();
+        let before = call.clone();
+
+        call.update(ToolCallUpdateFields::new());
+
+        assert_eq!(call, before);
+    }
+
+    #[test]
+    fn tool_call_update_overwrites_collections_instead_of_extending() {
+        // The docs on `ToolCallUpdate` promise: "Collections (content,
+        // locations) are overwritten, not extended." Lock that behavior in.
+        let mut call = sample_tool_call();
+        assert_eq!(call.content.len(), 1);
+        assert_eq!(call.locations.len(), 1);
+
+        call.update(
+            ToolCallUpdateFields::new()
+                .content(Vec::<ToolCallContent>::new())
+                .locations(Vec::<ToolCallLocation>::new()),
+        );
+
+        assert!(call.content.is_empty());
+        assert!(call.locations.is_empty());
+    }
+
+    #[test]
+    fn tool_call_update_does_not_clear_raw_input_when_field_absent() {
+        // `raw_input`/`raw_output` are `Option<Value>` on both sides. The
+        // update should only overwrite when a value is provided, otherwise
+        // agents that stream successive updates would lose earlier inputs.
+        let mut call = sample_tool_call();
+        assert!(call.raw_input.is_some());
+
+        call.update(ToolCallUpdateFields::new().status(ToolCallStatus::Completed));
+
+        assert_eq!(call.raw_input, Some(json!({ "path": "/tmp/a.txt" })));
+        assert_eq!(call.status, ToolCallStatus::Completed);
+    }
+
+    #[test]
+    fn tool_call_try_from_update_requires_title() {
+        let update = ToolCallUpdate::new("call-1", ToolCallUpdateFields::new());
+
+        let err = ToolCall::try_from(update).expect_err("expected InvalidParams");
+        assert_eq!(err.code, ErrorCode::InvalidParams);
+        assert_eq!(err.data, Some(json!("title is required for a tool call")));
+    }
+
+    #[test]
+    fn tool_call_try_from_update_uses_defaults_for_missing_fields() {
+        let update = ToolCallUpdate::new("call-1", ToolCallUpdateFields::new().title("Do a thing"));
+
+        let call = ToolCall::try_from(update).expect("should build");
+        assert_eq!(call.tool_call_id, ToolCallId::new("call-1"));
+        assert_eq!(call.title, "Do a thing");
+        assert_eq!(call.kind, ToolKind::default());
+        assert_eq!(call.status, ToolCallStatus::default());
+        assert!(call.content.is_empty());
+        assert!(call.locations.is_empty());
+        assert!(call.raw_input.is_none());
+        assert!(call.raw_output.is_none());
+        assert!(call.meta.is_none());
+    }
+
+    #[test]
+    fn tool_call_update_roundtrips_through_try_from() {
+        let original = sample_tool_call();
+        let update: ToolCallUpdate = original.clone().into();
+        let rebuilt = ToolCall::try_from(update).expect("should rebuild");
+        assert_eq!(rebuilt, original);
+    }
+
+    #[test]
+    fn tool_call_default_kind_and_status_are_skipped_on_serialization() {
+        // A freshly created ToolCall has default kind (Other) and status
+        // (Pending). Both should be omitted so we don't inflate the wire
+        // format for the common case and don't force older clients to
+        // handle the new fields.
+        let call = ToolCall::new("call-1", "Doing something");
+        let json = serde_json::to_value(&call).unwrap();
+        let obj = json.as_object().expect("expected object");
+        assert!(!obj.contains_key("kind"), "kind should be skipped: {json}");
+        assert!(
+            !obj.contains_key("status"),
+            "status should be skipped: {json}"
+        );
+        assert!(!obj.contains_key("content"));
+        assert!(!obj.contains_key("locations"));
+    }
+
+    #[test]
+    fn tool_call_non_default_status_is_serialized() {
+        let call = ToolCall::new("call-1", "Working").status(ToolCallStatus::InProgress);
+        let json = serde_json::to_value(&call).unwrap();
+        assert_eq!(json["status"], json!("in_progress"));
+    }
+
+    #[test]
+    fn tool_call_unknown_kind_falls_back_to_other() {
+        // `ToolKind` uses `#[serde(other)]` on `Other` so forward-compatible
+        // clients don't crash when the agent introduces a new kind.
+        let call: ToolCall = serde_json::from_value(json!({
+            "toolCallId": "call-1",
+            "title": "Future thing",
+            "kind": "totally_new_kind"
+        }))
+        .expect("should tolerate unknown kind");
+        assert_eq!(call.kind, ToolKind::Other);
+    }
+
+    #[test]
+    fn tool_call_update_skips_invalid_content_and_locations() {
+        // Both fields use `DefaultOnError<VecSkipError<...>>`, so malformed
+        // entries in a stream from a peer must not poison the whole update.
+        let raw = json!({
+            "toolCallId": "call-1",
+            "content": [
+                {
+                    "type": "content",
+                    "content": { "type": "text", "text": "ok" }
+                },
+                { "type": "definitely_not_a_variant" }
+            ],
+            "locations": [
+                { "path": "/tmp/a.txt" },
+                { "path": 123 }
+            ]
+        });
+
+        let update: ToolCallUpdate =
+            serde_json::from_value(raw).expect("update should tolerate garbage");
+        let content = update.fields.content.expect("content should decode");
+        assert_eq!(content.len(), 1, "invalid content entry should be skipped");
+
+        let locations = update.fields.locations.expect("locations should decode");
+        assert_eq!(
+            locations.len(),
+            1,
+            "invalid location entry should be skipped"
+        );
+        assert_eq!(locations[0].path, PathBuf::from("/tmp/a.txt"));
+    }
+}