fix(things): harden ToolEnvelope (optional summary, extra=ignore) (shelf) (#36)

Author: CaseyRo

src/things_mcp/models.py

@@ -30,16 +30,29 @@
 
 
 class ToolEnvelope(BaseModel, Generic[DataT]):
-    """Uniform JSON envelope returned by every tool's `structured_content`."""
+    """Uniform JSON envelope returned by every tool's `structured_content`.
+
+    Hardened to never raise at construction time on an internal path:
+
+    - ``summary`` is optional and defaults to ``""``. Every tool still sets a
+      real summary on the wire (the external return contract is unchanged), but
+      an internal caller that omits it gets an empty headline instead of a
+      runtime ``ValidationError``.
+    - ``extra="ignore"`` means an unexpected key (e.g. one re-attached by a
+      client/middleware round-trip) is dropped rather than rejected.
+      ``ClientCompatibilityMiddleware`` already strips extras on the wire, so
+      the shape clients see is unchanged; this only removes the footgun where a
+      stray key would crash construction.
+    """
 
-    model_config = ConfigDict(extra="forbid")
+    model_config = ConfigDict(extra="ignore")
 
     data: DataT | None = Field(
         default=None,
         description="Typed payload. Null when the tool's only useful signal is the summary.",
     )
     summary: str = Field(
-        ...,
+        default="",
         description="One-sentence machine-readable headline.",
     )
     meta: dict[str, Any] = Field(

tests/test_models.py

@@ -215,9 +215,43 @@ def test_envelope_meta_defaults_to_empty_dict(self):
         env = ToolEnvelope(data=None, summary="ok")
         assert env.meta == {}
 
-    def test_envelope_extra_fields_forbidden(self):
-        with pytest.raises(Exception):
-            ToolEnvelope.model_validate({"data": None, "summary": "ok", "extra": 1})
+    def test_envelope_summary_is_optional_and_defaults_to_empty(self):
+        """Hardened envelope: omitting `summary` must not raise (footgun fix).
+
+        Internal paths that forget to set a summary should get an empty
+        headline instead of a runtime ValidationError.
+        """
+        env = ToolEnvelope(data=None)
+        assert env.summary == ""
+        # Same via model_validate (the structured-output path).
+        validated = ToolEnvelope.model_validate({"data": None})
+        assert validated.summary == ""
+
+    def test_envelope_ignores_unexpected_extra_key(self):
+        """Hardened envelope: an unexpected extra key must be dropped, not raise.
+
+        ClientCompatibilityMiddleware already strips extras on the wire; this
+        guarantees the model itself degrades gracefully if one slips through.
+        """
+        # Construction with an unexpected kwarg must not raise.
+        env = ToolEnvelope(data=None, summary="ok", unexpected="boom")
+        assert env.summary == "ok"
+        assert not hasattr(env, "unexpected")
+        # And via model_validate (the round-trip path).
+        validated = ToolEnvelope.model_validate(
+            {"data": None, "summary": "ok", "extra": 1}
+        )
+        assert validated.summary == "ok"
+        # The extra key is ignored, so the serialized shape clients see is
+        # unchanged (data/summary/meta only).
+        assert set(validated.model_dump().keys()) == {"data", "summary", "meta"}
+
+    def test_envelope_no_args_does_not_raise(self):
+        """Belt-and-suspenders: a fully bare envelope is constructible."""
+        env = ToolEnvelope()
+        assert env.data is None
+        assert env.summary == ""
+        assert env.meta == {}
 
     def test_todo_status_enum_enforced(self):
         with pytest.raises(Exception):