cowork-bot: automated improvements (cowork/improve-api-contract-guardian)

README.md

@@ -12,7 +12,7 @@ Monitor OpenAPI schema diffs between git branches, detect breaking changes, gene
 |[![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](https://github.com/Coding-Dev-Tools/api-contract-guardian/blob/main/LICENSE)
 |[![Open Source Alternative](https://img.shields.io/badge/Open_Source_Alternative-%E2%87%92-blue?logo=opensourceinitiative)](https://www.opensourcealternative.to/project/api-contract-guardian)
 |[![LibHunt](https://img.shields.io/badge/LibHunt-%E2%87%92-blue?logo=codeigniter)](https://www.libhunt.com/r/Coding-Dev-Tools/api-contract-guardian)
-|[![PyPI](https://img.shields.io/pypi/v/api-contract-guardian)](https://pypi.org/project/api-contract-guardian/)
+|<!-- Not on public PyPI yet — install via git+; see Installation section -->
 
 **Why API Contract Guardian?**
 
@@ -24,11 +24,9 @@ Real-world scenarios:
 
 ## Installation
 
-```bash
-pip install api-contract-guardian
-```
+> **Note:** `api-contract-guardian` is not yet on public PyPI. Use one of the methods below.
 
-Or install the latest version directly from GitHub:
+Install the latest version directly from GitHub:
 
 ```bash
 pip install git+https://github.com/Coding-Dev-Tools/api-contract-guardian.git

src/api_contract_guardian/diff.py

@@ -963,7 +963,9 @@ def _diff_schema_details(
                         kind="property_format_changed",
                         severity=Severity.DANGEROUS,
                         path=f"{schema_path}.properties.{prop_name}",
-                        description=f"Property '{prop_name}' in '{name}' format changed from '{old_fmt}' to '{new_fmt}'",
+                        description=(
+                            f"Property '{prop_name}' in '{name}' format changed from '{old_fmt}' to '{new_fmt}'"
+                        ),
                         old_value=old_fmt,
                         new_value=new_fmt,
                     )
@@ -985,6 +987,110 @@ def _diff_schema_details(
                     )
                 )
 
+    # Deep structural diff of NESTED object / array-of-object properties.
+    # Previously _diff_schema_details only compared a component's DIRECT
+    # properties (type/format/enum); a nested object property whose sub-fields
+    # changed (e.g. a required field added deep inside) was reported as "no
+    # change" -- the silent-green failure class this tool exists to catch.
+    # Reuse the proven recursive inline-schema walker (conservative: treats all
+    # required/presence changes as BREAKING, since a component may back a request
+    # body). Only the nested sub-schemas are walked here, so this never
+    # double-reports the component's own top-level properties already checked above.
+    for prop_name in old_props:
+        if prop_name not in new_props:
+            continue
+        old_sub = old_props[prop_name] or {}
+        new_sub = new_props[prop_name] or {}
+        if not isinstance(old_sub, dict) or not isinstance(new_sub, dict):
+            continue
+        is_nested_object = (
+            (old_sub.get("type") == "object" and new_sub.get("type") == "object")
+            or ("properties" in old_sub and "properties" in new_sub)
+        )
+        if is_nested_object:
+            _diff_inline_schema(
+                f"{schema_path}.properties.{prop_name}",
+                old_sub,
+                new_sub,
+                result,
+                is_request=True,
+                depth=1,
+            )
+            continue
+        if old_sub.get("type") == "array" and new_sub.get("type") == "array":
+            old_items = old_sub.get("items") or {}
+            new_items = new_sub.get("items") or {}
+            if isinstance(old_items, dict) and isinstance(new_items, dict) and (
+                (old_items.get("type") == "object" and new_items.get("type") == "object")
+                or ("properties" in old_items and "properties" in new_items)
+            ):
+                _diff_inline_schema(
+                    f"{schema_path}.properties.{prop_name}.items",
+                    old_items,
+                    new_items,
+                    result,
+                    is_request=True,
+                    depth=1,
+                )
+
+    # A property that is a $ref (or array of $ref) into another component was
+    # previously ignored entirely: retargeting or dropping the ref produced no
+    # change record. Flag ref-target changes at the property's OWN path. The
+    # referenced target's internals are already diffed by _diff_schemas at
+    # components.schemas.<Target>, so this never double-reports them -- it only
+    # surfaces that THIS property's reference changed.
+    for prop_name in set(old_props) | set(new_props):
+        _flag_schema_ref_prop(
+            schema_path, prop_name, old_props.get(prop_name), new_props.get(prop_name), result
+        )
+
+
+def _flag_schema_ref_prop(
+    schema_path: str,
+    prop_name: str,
+    old_prop: Any,
+    new_prop: Any,
+    result: DiffResult,
+) -> None:
+    """Flag a component property whose ``$ref`` target changed or was dropped.
+
+    Refs into ``components.schemas`` are intentionally NOT followed into the
+    target's internals (``_diff_schemas`` already diffs the target at its own
+    path); we only report that the reference here changed, which is independent
+    information not otherwise surfaced.
+    """
+
+    def ref_of(sub: Any) -> Any:
+        if isinstance(sub, dict):
+            return sub.get("$ref")
+        return None
+
+    old_ref = ref_of(old_prop)
+    new_ref = ref_of(new_prop)
+    # Array whose items are a $ref.
+    if old_ref is None and isinstance(old_prop, dict) and old_prop.get("type") == "array":
+        old_ref = ref_of(old_prop.get("items"))
+    if new_ref is None and isinstance(new_prop, dict) and new_prop.get("type") == "array":
+        new_ref = ref_of(new_prop.get("items"))
+
+    if old_ref is None and new_ref is None:
+        return
+
+    prop_path = f"{schema_path}.properties.{prop_name}"
+    if old_ref != new_ref:
+        result.changes.append(
+            Change(
+                kind="schema_ref_changed",
+                severity=Severity.DANGEROUS,
+                path=prop_path,
+                description=(
+                    f"$ref property '{prop_name}' target changed from '{old_ref}' to '{new_ref}'"
+                ),
+                old_value=old_ref,
+                new_value=new_ref,
+            )
+        )
+
 
 def _diff_security_schemes(
     old: dict[str, Any],

tests/test_diff.py

@@ -663,6 +663,159 @@ def test_property_no_longer_required_non_breaking(self):
         result = diff_specs(old, new)
         assert any(c.kind == "property_no_longer_required" for c in result.changes)
 
+    def test_nested_object_property_required_change_is_breaking(self):
+        # A required field added DEEP inside a nested object property was
+        # previously reported as "no change" (silent-green gap at the
+        # component level). It must now surface as a breaking change.
+        old = _make_spec(
+            schemas={
+                "User": {
+                    "type": "object",
+                    "properties": {
+                        "id": {"type": "string"},
+                        "address": {
+                            "type": "object",
+                            "properties": {
+                                "street": {"type": "string"},
+                                "zip": {"type": "string"},
+                            },
+                        },
+                    },
+                }
+            }
+        )
+        new = _make_spec(
+            schemas={
+                "User": {
+                    "type": "object",
+                    "properties": {
+                        "id": {"type": "string"},
+                        "address": {
+                            "type": "object",
+                            "required": ["zip"],
+                            "properties": {
+                                "street": {"type": "string"},
+                                "zip": {"type": "string"},
+                            },
+                        },
+                    },
+                }
+            }
+        )
+        result = diff_specs(old, new)
+        became_required = [
+            c for c in result.changes if c.kind == "request_property_became_required"
+        ]
+        assert became_required, "nested required change must be detected"
+        assert any(
+            "address" in c.path and "zip" in c.path for c in became_required
+        ), "change must be reported at the nested path"
+        assert all(c.severity.value == "breaking" for c in became_required), (
+            "nested required change is breaking"
+        )
+
+    def test_nested_array_item_property_required_change_is_breaking(self):
+        old = _make_spec(
+            schemas={
+                "Cart": {
+                    "type": "object",
+                    "properties": {
+                        "items": {
+                            "type": "array",
+                            "items": {
+                                "type": "object",
+                                "properties": {"sku": {"type": "string"}},
+                            },
+                        },
+                    },
+                }
+            }
+        )
+        new = _make_spec(
+            schemas={
+                "Cart": {
+                    "type": "object",
+                    "properties": {
+                        "items": {
+                            "type": "array",
+                            "items": {
+                                "type": "object",
+                                "required": ["sku"],
+                                "properties": {"sku": {"type": "string"}},
+                            },
+                        },
+                    },
+                }
+            }
+        )
+        result = diff_specs(old, new)
+        became_required = [
+            c for c in result.changes if c.kind == "request_property_became_required"
+        ]
+        assert became_required, "nested array-item required change must be detected"
+        assert any("items" in c.path and "sku" in c.path for c in became_required)
+
+    def test_component_ref_property_target_change_is_dangerous(self):
+        # A component property whose $ref target was retargeted must be
+        # flagged as DANGEROUS (it was previously silently ignored at the
+        # component level).
+        old = _make_spec(
+            schemas={
+                "Order": {
+                    "type": "object",
+                    "properties": {
+                        "customer": {"$ref": "#/components/schemas/CustomerV1"},
+                    },
+                },
+                "CustomerV1": {"type": "object"},
+                "CustomerV2": {"type": "object"},
+            }
+        )
+        new = _make_spec(
+            schemas={
+                "Order": {
+                    "type": "object",
+                    "properties": {
+                        "customer": {"$ref": "#/components/schemas/CustomerV2"},
+                    },
+                },
+                "CustomerV1": {"type": "object"},
+                "CustomerV2": {"type": "object"},
+            }
+        )
+        result = diff_specs(old, new)
+        ref_changed = [c for c in result.changes if c.kind == "schema_ref_changed"]
+        assert ref_changed, "ref-target change must be detected"
+        assert any(c.severity.value == "dangerous" for c in ref_changed)
+        assert any("Order" in c.path and "customer" in c.path for c in ref_changed)
+
+    def test_component_ref_property_removed_is_dangerous(self):
+        old = _make_spec(
+            schemas={
+                "Order": {
+                    "type": "object",
+                    "properties": {
+                        "customer": {"$ref": "#/components/schemas/Customer"},
+                    },
+                },
+                "Customer": {"type": "object"},
+            }
+        )
+        new = _make_spec(
+            schemas={
+                "Order": {
+                    "type": "object",
+                    "properties": {
+                        "customer": {"type": "object"},
+                    },
+                },
+                "Customer": {"type": "object"},
+            }
+        )
+        result = diff_specs(old, new)
+        ref_changed = [c for c in result.changes if c.kind == "schema_ref_changed"]
+        assert ref_changed, "dropped $ref must be detected"
+
     def test_property_removed_breaking_if_required(self):
         old = _make_spec(
             schemas={