Skip to content

Commit 840f269

Browse files
cowork-botcowork-bot
committed
cowork-bot: recursively diff nested object/array-item schemas
Previously _diff_inline_schema only compared top-level properties, so a breaking change buried one level down (e.g. a required field added deep in a request body, or a field dropped from a nested response object / array-of-object items) was reported as 'no change' -- the silent-green failure class this tool exists to catch. Now recurse into nested object properties and array-of-object item schemas with the same request/response-aware breaking semantics, guarded by a depth limit and skipping nested $ref targets (component-level diffing owns those). +4 regression tests (nested response field removal, nested required request field, array-item field removal, and a no-false-positive guard); 188 tests pass, ruff clean.
1 parent b489730 commit 840f269

3 files changed

Lines changed: 237 additions & 0 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ All notable changes to API Contract Guardian will be documented in this file.
77
### Added
88

99
- Operation-level (per-endpoint) `security` diffing: detects when an endpoint drops its auth requirement (becomes public), newly requires authentication, or switches security schemes — surfaced as DANGEROUS changes (previously only global `security` was compared)
10+
- Recursive nested schema diffing: breaking changes inside nested object properties and array-of-object `items` schemas are now detected (e.g. a required field added deep in a request body, or a field dropped from a nested response object) — previously only top-level properties were compared, so a change buried one level down was reported as "no change"
1011
- MCP server integration via `mcp` subcommand (#6)
1112
- GitHub Pages deployment workflow (`pages.yml`)
1213
- npm-publish workflow for npm publishing

‎src/api_contract_guardian/diff.py‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -618,6 +618,7 @@ def _diff_inline_schema(
618618
result: DiffResult,
619619
*,
620620
is_request: bool,
621+
depth: int = 0,
621622
) -> None:
622623
"""Diff two inline schemas with request/response-aware breaking semantics.
623624
@@ -741,6 +742,63 @@ def _diff_inline_schema(
741742
)
742743
)
743744

745+
# Recurse into nested object properties and array-of-object item schemas so
746+
# breaking changes buried inside a nested object/array are not reported as
747+
# "no change" (the silent-green failure class this tool exists to catch).
748+
# Component-level $ref targets are diffed by _diff_schemas, so a nested $ref
749+
# property is intentionally NOT recursed here.
750+
_MAX_SCHEMA_DEPTH = 6
751+
if depth < _MAX_SCHEMA_DEPTH:
752+
for prop_name in old_props:
753+
if prop_name not in new_props:
754+
continue
755+
old_prop = old_props[prop_name] or {}
756+
new_prop = new_props[prop_name] or {}
757+
if not isinstance(old_prop, dict) or not isinstance(new_prop, dict):
758+
continue
759+
# Nested object -> recurse into its own properties/required set.
760+
both_object = (
761+
old_prop.get("type") == "object" and new_prop.get("type") == "object"
762+
) or ("properties" in old_prop and "properties" in new_prop)
763+
if both_object:
764+
_diff_inline_schema(
765+
f"{path}.properties.{prop_name}",
766+
old_prop,
767+
new_prop,
768+
result,
769+
is_request=is_request,
770+
depth=depth + 1,
771+
)
772+
continue
773+
# Array whose items are an object -> recurse into the item schema.
774+
if (
775+
old_prop.get("type") == "array"
776+
and new_prop.get("type") == "array"
777+
):
778+
old_items = old_prop.get("items") or {}
779+
new_items = new_prop.get("items") or {}
780+
if (
781+
isinstance(old_items, dict)
782+
and isinstance(new_items, dict)
783+
and (
784+
(
785+
old_items.get("type") == "object"
786+
and new_items.get("type") == "object"
787+
)
788+
or (
789+
"properties" in old_items and "properties" in new_items
790+
)
791+
)
792+
):
793+
_diff_inline_schema(
794+
f"{path}.properties.{prop_name}.items",
795+
old_items,
796+
new_items,
797+
result,
798+
is_request=is_request,
799+
depth=depth + 1,
800+
)
801+
744802

745803
def _diff_schemas(old: dict[str, Any], new: dict[str, Any], result: DiffResult) -> None:
746804
"""Detect schema/component changes."""

‎tests/test_diff.py‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1576,3 +1576,181 @@ def test_identical_inline_schemas_no_change(self):
15761576
"response_property_no_longer_required",
15771577
}
15781578
assert not any(c.kind in schema_kinds for c in result.changes)
1579+
1580+
def test_nested_object_response_property_removed_is_breaking(self):
1581+
old = _make_spec(
1582+
paths={
1583+
"/u": {
1584+
"get": _op_body(
1585+
response_schema={
1586+
"type": "object",
1587+
"properties": {
1588+
"id": {"type": "string"},
1589+
"address": {
1590+
"type": "object",
1591+
"properties": {
1592+
"street": {"type": "string"},
1593+
"city": {"type": "string"},
1594+
},
1595+
},
1596+
},
1597+
}
1598+
)
1599+
}
1600+
}
1601+
)
1602+
new = _make_spec(
1603+
paths={
1604+
"/u": {
1605+
"get": _op_body(
1606+
response_schema={
1607+
"type": "object",
1608+
"properties": {
1609+
"id": {"type": "string"},
1610+
"address": {
1611+
"type": "object",
1612+
"properties": {"street": {"type": "string"}},
1613+
},
1614+
},
1615+
}
1616+
)
1617+
}
1618+
}
1619+
)
1620+
result = diff_specs(old, new)
1621+
matches = [c for c in result.changes if c.kind == "response_property_removed"]
1622+
assert matches, "dropping a nested response field must be flagged"
1623+
assert matches[0].severity == Severity.BREAKING
1624+
assert "address.properties.city" in matches[0].path
1625+
1626+
def test_nested_object_request_new_required_field_is_breaking(self):
1627+
old = _make_spec(
1628+
paths={
1629+
"/u": {
1630+
"post": _op_body(
1631+
request_schema={
1632+
"type": "object",
1633+
"properties": {
1634+
"profile": {
1635+
"type": "object",
1636+
"properties": {"name": {"type": "string"}},
1637+
}
1638+
},
1639+
}
1640+
)
1641+
}
1642+
}
1643+
)
1644+
new = _make_spec(
1645+
paths={
1646+
"/u": {
1647+
"post": _op_body(
1648+
request_schema={
1649+
"type": "object",
1650+
"properties": {
1651+
"profile": {
1652+
"type": "object",
1653+
"properties": {
1654+
"name": {"type": "string"},
1655+
"bio": {"type": "string"},
1656+
},
1657+
"required": ["name", "bio"],
1658+
}
1659+
},
1660+
}
1661+
)
1662+
}
1663+
}
1664+
)
1665+
result = diff_specs(old, new)
1666+
matches = [c for c in result.changes if c.kind == "request_property_added"]
1667+
assert matches, "adding a required nested request field must be flagged"
1668+
assert matches[0].severity == Severity.BREAKING
1669+
assert "profile.properties.bio" in matches[0].path
1670+
assert "(required)" in matches[0].description
1671+
1672+
def test_nested_array_item_property_removed_is_breaking(self):
1673+
old = _make_spec(
1674+
paths={
1675+
"/u": {
1676+
"get": _op_body(
1677+
response_schema={
1678+
"type": "object",
1679+
"properties": {
1680+
"items": {
1681+
"type": "array",
1682+
"items": {
1683+
"type": "object",
1684+
"properties": {
1685+
"sku": {"type": "string"},
1686+
"qty": {"type": "integer"},
1687+
},
1688+
},
1689+
}
1690+
},
1691+
}
1692+
)
1693+
}
1694+
}
1695+
)
1696+
new = _make_spec(
1697+
paths={
1698+
"/u": {
1699+
"get": _op_body(
1700+
response_schema={
1701+
"type": "object",
1702+
"properties": {
1703+
"items": {
1704+
"type": "array",
1705+
"items": {
1706+
"type": "object",
1707+
"properties": {"qty": {"type": "integer"}},
1708+
},
1709+
}
1710+
},
1711+
}
1712+
)
1713+
}
1714+
}
1715+
)
1716+
result = diff_specs(old, new)
1717+
matches = [c for c in result.changes if c.kind == "response_property_removed"]
1718+
assert matches, "dropping a field inside an array-of-objects must be flagged"
1719+
assert matches[0].severity == Severity.BREAKING
1720+
assert "items.items.properties.sku" in matches[0].path
1721+
1722+
def test_identical_nested_schemas_no_change(self):
1723+
schema = {
1724+
"type": "object",
1725+
"properties": {
1726+
"id": {"type": "string"},
1727+
"address": {
1728+
"type": "object",
1729+
"properties": {"city": {"type": "string"}},
1730+
"required": ["city"],
1731+
},
1732+
"items": {
1733+
"type": "array",
1734+
"items": {
1735+
"type": "object",
1736+
"properties": {"sku": {"type": "string"}},
1737+
},
1738+
},
1739+
},
1740+
}
1741+
old = _make_spec(
1742+
paths={"/u": {"post": _op_body(request_schema=schema, response_schema=schema)}}
1743+
)
1744+
new = _make_spec(
1745+
paths={"/u": {"post": _op_body(request_schema=schema, response_schema=schema)}}
1746+
)
1747+
result = diff_specs(old, new)
1748+
nested_kinds = {
1749+
"schema_type_changed",
1750+
"property_type_changed",
1751+
"request_property_added",
1752+
"response_property_removed",
1753+
"request_property_became_required",
1754+
"response_property_no_longer_required",
1755+
}
1756+
assert not any(c.kind in nested_kinds for c in result.changes)

0 commit comments

Comments
 (0)