ci: use oasdiff for agent-server REST API breakage detection

.github/scripts/check_agent_server_rest_api_breakage.py

@@ -1,8 +1,8 @@
 #!/usr/bin/env python3
-"""REST API breakage detection for openhands-agent-server.
+"""REST API breakage detection for openhands-agent-server using oasdiff.
 
 This script compares the current OpenAPI schema for the agent-server REST API against
-the previous published version on PyPI.
+the previous published version on PyPI, using oasdiff for breaking change detection.
 
 Policies enforced (mirrors the SDK's Griffe checks, but for REST):
 
@@ -14,13 +14,6 @@
    - If a breaking REST change is detected, the current version must be at least a
      MINOR bump compared to the previous release.
 
-The breakage detection currently focuses on compatibility rules that are robust to
-OpenAPI generation changes:
-- Removed operations
-- New required parameters
-- Request bodies that became required
-- New required fields in JSON request bodies (best-effort)
-
 If the previous release schema can't be fetched (e.g., network / PyPI issues), the
 script emits a warning and exits successfully to avoid flaky CI.
 """
@@ -33,8 +26,6 @@
 import tempfile
 import tomllib
 import urllib.request
-from collections.abc import Iterable
-from dataclasses import dataclass
 from pathlib import Path
 
 from packaging import version as pkg_version
@@ -45,24 +36,6 @@
 PYPI_DISTRIBUTION = "openhands-agent-server"
 
 
-_HTTP_METHODS = (
-    "get",
-    "post",
-    "put",
-    "patch",
-    "delete",
-    "options",
-    "head",
-    "trace",
-)
-
-
-@dataclass(frozen=True, slots=True)
-class OperationKey:
-    method: str
-    path: str
-
-
 def _read_version_from_pyproject(pyproject: Path) -> str:
     data = tomllib.loads(pyproject.read_text())
     try:
@@ -186,103 +159,43 @@ def _generate_openapi_for_version(version: str) -> dict | None:
             return None
 
 
-def _iter_operations(schema: dict) -> Iterable[tuple[OperationKey, dict]]:
-    paths: dict = schema.get("paths", {})
-    for path, path_item in paths.items():
-        if not isinstance(path_item, dict):
-            continue
-        for method in _HTTP_METHODS:
-            operation = path_item.get(method)
-            if isinstance(operation, dict):
-                yield OperationKey(method=method, path=path), operation
-
-
-def _required_parameters(operation: dict) -> set[tuple[str, str]]:
-    required: set[tuple[str, str]] = set()
-    for param in operation.get("parameters", []) or []:
-        if not isinstance(param, dict):
-            continue
-        if not param.get("required"):
-            continue
-        name = param.get("name")
-        location = param.get("in")
-        if isinstance(name, str) and isinstance(location, str):
-            required.add((name, location))
-    return required
-
-
-def _resolve_ref(schema: dict, spec: dict, *, max_depth: int = 50) -> dict:
-    current = schema
-    seen: set[str] = set()
-    depth = 0
-
-    while isinstance(current, dict) and "$ref" in current:
-        ref = current["$ref"]
-        if not isinstance(ref, str) or not ref.startswith("#/"):
-            return current
-        if ref in seen or depth >= max_depth:
-            return current
-
-        seen.add(ref)
-        depth += 1
-
-        target: object = spec
-        for part in ref.removeprefix("#/").split("/"):
-            if not isinstance(target, dict) or part not in target:
-                return current
-            target = target[part]
-        if not isinstance(target, dict):
-            return current
-        current = target
-
-    return current
-
-
-def _required_json_fields(operation: dict, spec: dict) -> set[str]:
-    request_body = operation.get("requestBody") or {}
-    if not isinstance(request_body, dict):
-        return set()
+def _run_oasdiff_breakage_check(
+    prev_spec: Path, cur_spec: Path
+) -> tuple[list[dict], int]:
+    """Run oasdiff breaking check between two OpenAPI specs.
 
-    content = request_body.get("content") or {}
-    if not isinstance(content, dict):
-        return set()
-
-    json_content = content.get("application/json")
-    if not isinstance(json_content, dict):
-        return set()
-
-    schema = json_content.get("schema")
-    if not isinstance(schema, dict):
-        return set()
-
-    return _required_json_fields_from_schema(schema, spec)
-
-
-def _required_json_fields_from_schema(schema: dict, spec: dict) -> set[str]:
-    resolved = _resolve_ref(schema, spec)
-
-    if "allOf" in resolved and isinstance(resolved["allOf"], list):
-        required: set[str] = set()
-        for item in resolved["allOf"]:
-            if isinstance(item, dict):
-                required |= _required_json_fields_from_schema(item, spec)
-        return required
-
-    if resolved.get("type") != "object":
-        return set()
-
-    required = resolved.get("required")
-    if not isinstance(required, list):
-        return set()
-
-    return {field for field in required if isinstance(field, str)}
+    Returns (list of breaking changes, exit code from oasdiff).
+    """
+    try:
+        result = subprocess.run(
+            [
+                "oasdiff",
+                "breaking",
+                "-f",
+                "json",
+                "--fail-on",
+                "ERR",
+                str(prev_spec),
+                str(cur_spec),
+            ],
+            capture_output=True,
+            text=True,
+        )
+    except FileNotFoundError:
+        print(
+            "::warning title=oasdiff not found::"
+            "Please install oasdiff: https://github.com/oasdiff/oasdiff"
+        )
+        return [], 0
 
+    breaking_changes = []
+    if result.stdout:
+        try:
+            breaking_changes = json.loads(result.stdout)
+        except json.JSONDecodeError:
+            pass
 
-def _is_request_body_required(operation: dict) -> bool:
-    request_body = operation.get("requestBody")
-    if not isinstance(request_body, dict):
-        return False
-    return bool(request_body.get("required"))
+    return breaking_changes, result.returncode
 
 
 def _is_minor_or_major_bump(current: str, previous: str) -> bool:
@@ -293,58 +206,6 @@ def _is_minor_or_major_bump(current: str, previous: str) -> bool:
     return (cur.major, cur.minor) != (prev.major, prev.minor)
 
 
-def _compute_breakages(
-    prev_schema: dict, current_schema: dict
-) -> tuple[list[str], list[OperationKey]]:
-    prev_ops = dict(_iter_operations(prev_schema))
-    cur_ops = dict(_iter_operations(current_schema))
-
-    removed = set(prev_ops).difference(cur_ops)
-
-    undeprecated_removals: list[OperationKey] = []
-    for key in sorted(removed, key=lambda k: (k.path, k.method)):
-        if not prev_ops[key].get("deprecated"):
-            undeprecated_removals.append(key)
-
-    breaking_reasons: list[str] = []
-
-    if removed:
-        breaking_reasons.append(f"Removed operations: {len(removed)}")
-
-    for key, prev_op in prev_ops.items():
-        cur_op = cur_ops.get(key)
-        if cur_op is None:
-            continue
-
-        new_required_params = _required_parameters(cur_op) - _required_parameters(
-            prev_op
-        )
-        if new_required_params:
-            formatted = ", ".join(
-                sorted(f"{n}({loc})" for n, loc in new_required_params)
-            )
-            breaking_reasons.append(
-                f"{key.method.upper()} {key.path}: new required params: {formatted}"
-            )
-
-        if _is_request_body_required(cur_op) and not _is_request_body_required(prev_op):
-            breaking_reasons.append(
-                f"{key.method.upper()} {key.path}: request body became required"
-            )
-
-        prev_required_fields = _required_json_fields(prev_op, prev_schema)
-        cur_required_fields = _required_json_fields(cur_op, current_schema)
-        new_required_fields = cur_required_fields - prev_required_fields
-        if new_required_fields:
-            formatted = ", ".join(sorted(new_required_fields))
-            breaking_reasons.append(
-                f"{key.method.upper()} {key.path}: "
-                f"new required JSON fields: {formatted}"
-            )
-
-    return breaking_reasons, undeprecated_removals
-
-
 def main() -> int:
     current_version = _read_version_from_pyproject(AGENT_SERVER_PYPROJECT)
     prev_version = _get_previous_version(PYPI_DISTRIBUTION, current_version)
@@ -362,34 +223,81 @@ def main() -> int:
 
     current_schema = _generate_current_openapi()
 
-    breaking_reasons, undeprecated_removals = _compute_breakages(
-        prev_schema, current_schema
-    )
+    with tempfile.TemporaryDirectory(prefix="oasdiff-specs-") as tmp:
+        tmp_path = Path(tmp)
+        prev_spec_file = tmp_path / "prev_spec.json"
+        cur_spec_file = tmp_path / "cur_spec.json"
+
+        prev_spec_file.write_text(json.dumps(prev_schema, indent=2))
+        cur_spec_file.write_text(json.dumps(current_schema, indent=2))
+
+        breaking_changes, exit_code = _run_oasdiff_breakage_check(
+            prev_spec_file, cur_spec_file
+        )
+
+    if not breaking_changes:
+        if exit_code == 0:
+            print("No breaking changes detected.")
+        else:
+            print(
+                f"oasdiff returned exit code {exit_code} but no breaking changes "
+                "in JSON format. There may be warnings only."
+            )
+        return 0
+
+    removed_operations = []
+    other_breakage = []
+
+    for change in breaking_changes:
+        change_id = change.get("id", "")
+        text = change.get("text", "")
+        details = change.get("details", {})
+
+        if "removed" in change_id.lower() and "operation" in change_id.lower():
+            path = details.get("path", "")
+            method = details.get("method", "")
+            operation_id = details.get("operationId", "")
+            deprecated = details.get("deprecated", False)
+
+            removed_operations.append(
+                {
+                    "path": path,
+                    "method": method,
+                    "operationId": operation_id,
+                    "deprecated": deprecated,
+                }
+            )
+        else:
+            other_breakage.append(text)
+
+    undeprecated_removals = [
+        op for op in removed_operations if not op.get("deprecated", False)
+    ]
 
     if undeprecated_removals:
-        for key in undeprecated_removals:
+        for op in undeprecated_removals:
             print(
-                "::error "
-                f"title={PYPI_DISTRIBUTION} REST API::Removed {key.method.upper()} "
-                f"{key.path} without prior deprecation (deprecated=true)."
+                f"::error "
+                f"title={PYPI_DISTRIBUTION} REST API::Removed {op['method'].upper()} "
+                f"{op['path']} without prior deprecation (deprecated=true)."
             )
 
-    breaking = bool(breaking_reasons)
+    has_breaking = bool(breaking_changes)
 
-    if breaking and not _is_minor_or_major_bump(current_version, prev_version):
+    if has_breaking and not _is_minor_or_major_bump(current_version, prev_version):
         print(
             "::error "
             f"title={PYPI_DISTRIBUTION} REST API::Breaking REST API change detected "
             f"without MINOR version bump ({prev_version} -> {current_version})."
         )
 
-    if breaking:
-        print("Breaking REST API changes detected compared to previous release:")
-        for reason in breaking_reasons:
-            print(f"- {reason}")
+    if has_breaking:
+        print("\nBreaking REST API changes detected compared to previous release:")
+        for text in breaking_changes:
+            print(f"- {text.get('text', str(text))}")
 
     errors = bool(undeprecated_removals) or (
-        breaking and not _is_minor_or_major_bump(current_version, prev_version)
+        has_breaking and not _is_minor_or_major_bump(current_version, prev_version)
     )
     return 1 if errors else 0
 

.github/workflows/agent-server-rest-api-breakage.yml

@@ -26,6 +26,11 @@ jobs:
             - name: Install workspace deps (dev)
               run: uv sync --frozen --group dev
 
+            - name: Install oasdiff
+              run: |
+                  curl -L https://raw.githubusercontent.com/oasdiff/oasdiff/main/install.sh | sh -s -- -b /usr/local/bin
+                  oasdiff --version
+
             - name: Run agent server REST API breakage check
               id: api_breakage
               # Release PRs (rel-*) must block on breakage.

openhands-agent-server/AGENTS.md

@@ -43,7 +43,7 @@ Removing an endpoint is a breaking change.
 ### CI enforcement
 
 The workflow `Agent server REST API breakage checks` compares the current OpenAPI
-schema against the previous `openhands-agent-server` release on PyPI.
+schema against the previous `openhands-agent-server` release on PyPI using [oasdiff](https://github.com/oasdiff/oasdiff).
 
 It currently enforces:
 - No removal of operations (path + method) unless they were already marked