test: cover plan, tool_call, ext, and protocol_level regression risks

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/ext.rs

@@ -86,3 +86,64 @@ impl ExtNotification {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use serde_json::value::RawValue;
+
+    fn raw(s: &str) -> Arc<RawValue> {
+        RawValue::from_string(s.to_string()).unwrap().into()
+    }
+
+    #[test]
+    fn ext_request_new_preserves_method_verbatim() {
+        // Constructor accepts anything `Into<Arc<str>>`. The method name must
+        // be stored exactly as given so the routing layer can dispatch it.
+        let req = ExtRequest::new("_vendor/custom_action", raw(r#"{"x":1}"#));
+        assert_eq!(req.method.as_ref(), "_vendor/custom_action");
+
+        let req = ExtRequest::new(String::from("_other"), raw("null"));
+        assert_eq!(req.method.as_ref(), "_other");
+
+        let arc: Arc<str> = Arc::from("_keep");
+        let req = ExtRequest::new(arc.clone(), raw("[]"));
+        assert!(Arc::ptr_eq(&req.method, &arc), "Arc<str> should be reused");
+    }
+
+    #[test]
+    fn ext_request_serializes_only_params_not_method() {
+        // `#[serde(transparent)]` + `#[serde(skip)]` on method means the JSON
+        // surface is exactly the params blob. Emitting the method name into
+        // the params payload would break custom RPC handlers.
+        let req = ExtRequest::new("_vendor/x", raw(r#"{"answer":42}"#));
+        let serialized = serde_json::to_value(&req).unwrap();
+        assert_eq!(serialized, serde_json::json!({"answer": 42}));
+        assert!(
+            !serialized.to_string().contains("_vendor/x"),
+            "method name must not leak into the serialized params"
+        );
+    }
+
+    #[test]
+    fn ext_notification_serializes_only_params_not_method() {
+        let note = ExtNotification::new("_telemetry/event", raw(r#"{"k":"v"}"#));
+        let serialized = serde_json::to_value(&note).unwrap();
+        assert_eq!(serialized, serde_json::json!({"k": "v"}));
+    }
+
+    #[test]
+    fn ext_response_serializes_transparently() {
+        // ExtResponse is a thin wrapper; the wire format must equal its inner
+        // RawValue with no envelope, otherwise extension responses would not
+        // be assignable to a typed result on the client side.
+        let resp = ExtResponse::new(raw(r#"{"ok":true}"#));
+        let serialized = serde_json::to_value(&resp).unwrap();
+        assert_eq!(serialized, serde_json::json!({"ok": true}));
+
+        // The From<Arc<RawValue>> impl also works.
+        let from_impl: ExtResponse = raw("42").into();
+        let serialized = serde_json::to_value(&from_impl).unwrap();
+        assert_eq!(serialized, serde_json::json!(42));
+    }
+}

src/v1/plan.rs

@@ -145,3 +145,120 @@ pub enum PlanEntryStatus {
     /// The task has been successfully completed.
     Completed,
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use serde_json::json;
+
+    fn entry() -> PlanEntry {
+        PlanEntry::new("step", PlanEntryPriority::Medium, PlanEntryStatus::Pending)
+    }
+
+    #[test]
+    fn plan_entry_roundtrip_omits_optional_meta() {
+        let value = entry();
+        let serialized = serde_json::to_value(&value).unwrap();
+        assert_eq!(
+            serialized,
+            json!({
+                "content": "step",
+                "priority": "medium",
+                "status": "pending",
+            })
+        );
+        let parsed: PlanEntry = serde_json::from_value(serialized).unwrap();
+        assert_eq!(parsed, value);
+    }
+
+    #[test]
+    fn plan_entry_priority_and_status_use_snake_case() {
+        assert_eq!(
+            serde_json::to_value(&PlanEntryPriority::High).unwrap(),
+            json!("high"),
+        );
+        assert_eq!(
+            serde_json::to_value(&PlanEntryPriority::Low).unwrap(),
+            json!("low"),
+        );
+        assert_eq!(
+            serde_json::to_value(&PlanEntryStatus::InProgress).unwrap(),
+            json!("in_progress"),
+        );
+        assert_eq!(
+            serde_json::to_value(&PlanEntryStatus::Completed).unwrap(),
+            json!("completed"),
+        );
+
+        let parsed: PlanEntryPriority = serde_json::from_str("\"high\"").unwrap();
+        assert_eq!(parsed, PlanEntryPriority::High);
+        let parsed: PlanEntryStatus = serde_json::from_str("\"in_progress\"").unwrap();
+        assert_eq!(parsed, PlanEntryStatus::InProgress);
+    }
+
+    #[test]
+    fn plan_skips_malformed_entries_keeping_valid_ones() {
+        // Mirrors the `DefaultOnError<VecSkipError<_, SkipListener>>` pattern
+        // applied to `Plan::entries` so a single bad entry does not poison the
+        // whole plan update.
+        let input = json!({
+            "entries": [
+                {"content": "ok 1", "priority": "high", "status": "pending"},
+                {"content": "missing priority", "status": "pending"},
+                {"content": "wrong types", "priority": 7, "status": false},
+                "not even an object",
+                {"content": "ok 2", "priority": "low", "status": "completed"},
+            ]
+        });
+
+        let plan: Plan = serde_json::from_value(input).unwrap();
+        assert_eq!(plan.entries.len(), 2);
+        assert_eq!(plan.entries[0].content, "ok 1");
+        assert_eq!(plan.entries[0].priority, PlanEntryPriority::High);
+        assert_eq!(plan.entries[1].content, "ok 2");
+        assert_eq!(plan.entries[1].status, PlanEntryStatus::Completed);
+    }
+
+    #[test]
+    fn plan_collapses_outer_shape_errors_to_empty_entries() {
+        // `DefaultOnError` should swallow outer-shape failures so a producer
+        // sending the wrong type for `entries` does not break consumers.
+        // `entries` is required on the wire (no `#[serde(default)]`), so a
+        // missing key still errors; that's intentional and documented here.
+        let cases = [
+            json!({ "entries": null }),
+            json!({ "entries": "oops" }),
+            json!({ "entries": {"k": 1} }),
+            json!({ "entries": 42 }),
+        ];
+        for input in cases {
+            let plan: Plan = serde_json::from_value(input.clone()).unwrap_or_else(|e| {
+                panic!("expected Plan to deserialize from {input}: {e}");
+            });
+            assert!(
+                plan.entries.is_empty(),
+                "expected empty entries for {input}, got {:?}",
+                plan.entries
+            );
+        }
+
+        // Missing key is still an error - the field is required.
+        let err = serde_json::from_value::<Plan>(json!({})).unwrap_err();
+        assert!(
+            err.to_string().contains("entries"),
+            "expected missing-field error to mention `entries`, got: {err}"
+        );
+    }
+
+    #[test]
+    fn plan_preserves_meta_when_present() {
+        let mut meta = Meta::new();
+        meta.insert("k".to_string(), json!("v"));
+        let value = Plan::new(vec![entry()]).meta(meta.clone());
+
+        let serialized = serde_json::to_value(&value).unwrap();
+        assert_eq!(serialized["_meta"], json!({"k": "v"}));
+        let parsed: Plan = serde_json::from_value(serialized).unwrap();
+        assert_eq!(parsed.meta, Some(meta));
+    }
+}

src/v1/protocol_level.rs

@@ -120,3 +120,76 @@ impl ProtocolLevelNotification {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::v1::RequestId;
+    use serde_json::json;
+
+    #[test]
+    fn cancel_request_method_name_is_dollar_namespace() {
+        // `$/cancel_request` is the protocol-level method name that all sides
+        // are expected to recognize (or safely ignore). Renaming this is a
+        // wire-format break.
+        assert_eq!(CANCEL_REQUEST_METHOD_NAME, "$/cancel_request");
+        assert_eq!(
+            PROTOCOL_LEVEL_METHOD_NAMES.cancel_request,
+            CANCEL_REQUEST_METHOD_NAME
+        );
+    }
+
+    #[test]
+    fn protocol_level_notification_method_returns_cancel_request_name() {
+        let notif = ProtocolLevelNotification::CancelRequestNotification(
+            CancelRequestNotification::new(RequestId::Number(7)),
+        );
+        assert_eq!(notif.method(), CANCEL_REQUEST_METHOD_NAME);
+    }
+
+    #[test]
+    fn cancel_request_notification_roundtrip_omits_meta_when_unset() {
+        let notif = CancelRequestNotification::new(RequestId::Str("req_1".to_string()));
+        let serialized = serde_json::to_value(&notif).unwrap();
+        assert_eq!(serialized, json!({"requestId": "req_1"}));
+
+        let parsed: CancelRequestNotification = serde_json::from_value(serialized).unwrap();
+        assert_eq!(parsed, notif);
+    }
+
+    #[test]
+    fn cancel_request_notification_preserves_meta_when_set() {
+        let mut meta = Meta::new();
+        meta.insert("trace".to_string(), json!("abc123"));
+        let notif = CancelRequestNotification::new(RequestId::Number(42)).meta(meta.clone());
+
+        let serialized = serde_json::to_value(&notif).unwrap();
+        assert_eq!(
+            serialized,
+            json!({"requestId": 42, "_meta": {"trace": "abc123"}})
+        );
+
+        let parsed: CancelRequestNotification = serde_json::from_value(serialized).unwrap();
+        assert_eq!(parsed.meta, Some(meta));
+    }
+
+    #[test]
+    fn cancel_request_accepts_all_request_id_shapes() {
+        // RequestId is untagged: null / number / string. The cancellation
+        // notification must round-trip any of them so cancellation works
+        // regardless of how the original request was issued.
+        let cases = [
+            (RequestId::Null, json!(null)),
+            (RequestId::Number(0), json!(0)),
+            (RequestId::Number(-1), json!(-1)),
+            (RequestId::Str("hex-id".to_string()), json!("hex-id")),
+        ];
+        for (id, expected_id_json) in cases {
+            let notif = CancelRequestNotification::new(id.clone());
+            let serialized = serde_json::to_value(&notif).unwrap();
+            assert_eq!(serialized["requestId"], expected_id_json, "id: {id:?}");
+            let parsed: CancelRequestNotification = serde_json::from_value(serialized).unwrap();
+            assert_eq!(parsed.request_id, id);
+        }
+    }
+}