Skip to content

Commit 72512f8

Browse files
ci: use oasdiff for agent-server REST API breakage detection
Replaced custom Python diffing logic with oasdiff for OpenAPI breaking change detection. The script now: - Uses oasdiff CLI for detecting breaking changes between OpenAPI specs - Still enforces deprecation-before-removal and MINOR version bump policies - Keeps the logic for fetching previous version from PyPI and generating OpenAPI schemas for comparison Co-authored-by: openhands <openhands@all-hands.dev>
1 parent cefaebf commit 72512f8

File tree

3 files changed

+103
-190
lines changed

3 files changed

+103
-190
lines changed

.github/scripts/check_agent_server_rest_api_breakage.py

Lines changed: 97 additions & 189 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
#!/usr/bin/env python3
2-
"""REST API breakage detection for openhands-agent-server.
2+
"""REST API breakage detection for openhands-agent-server using oasdiff.
33
44
This script compares the current OpenAPI schema for the agent-server REST API against
5-
the previous published version on PyPI.
5+
the previous published version on PyPI, using oasdiff for breaking change detection.
66
77
Policies enforced (mirrors the SDK's Griffe checks, but for REST):
88
@@ -14,13 +14,6 @@
1414
- If a breaking REST change is detected, the current version must be at least a
1515
MINOR bump compared to the previous release.
1616
17-
The breakage detection currently focuses on compatibility rules that are robust to
18-
OpenAPI generation changes:
19-
- Removed operations
20-
- New required parameters
21-
- Request bodies that became required
22-
- New required fields in JSON request bodies (best-effort)
23-
2417
If the previous release schema can't be fetched (e.g., network / PyPI issues), the
2518
script emits a warning and exits successfully to avoid flaky CI.
2619
"""
@@ -33,8 +26,6 @@
3326
import tempfile
3427
import tomllib
3528
import urllib.request
36-
from collections.abc import Iterable
37-
from dataclasses import dataclass
3829
from pathlib import Path
3930

4031
from packaging import version as pkg_version
@@ -45,24 +36,6 @@
4536
PYPI_DISTRIBUTION = "openhands-agent-server"
4637

4738

48-
_HTTP_METHODS = (
49-
"get",
50-
"post",
51-
"put",
52-
"patch",
53-
"delete",
54-
"options",
55-
"head",
56-
"trace",
57-
)
58-
59-
60-
@dataclass(frozen=True, slots=True)
61-
class OperationKey:
62-
method: str
63-
path: str
64-
65-
6639
def _read_version_from_pyproject(pyproject: Path) -> str:
6740
data = tomllib.loads(pyproject.read_text())
6841
try:
@@ -186,103 +159,43 @@ def _generate_openapi_for_version(version: str) -> dict | None:
186159
return None
187160

188161

189-
def _iter_operations(schema: dict) -> Iterable[tuple[OperationKey, dict]]:
190-
paths: dict = schema.get("paths", {})
191-
for path, path_item in paths.items():
192-
if not isinstance(path_item, dict):
193-
continue
194-
for method in _HTTP_METHODS:
195-
operation = path_item.get(method)
196-
if isinstance(operation, dict):
197-
yield OperationKey(method=method, path=path), operation
198-
199-
200-
def _required_parameters(operation: dict) -> set[tuple[str, str]]:
201-
required: set[tuple[str, str]] = set()
202-
for param in operation.get("parameters", []) or []:
203-
if not isinstance(param, dict):
204-
continue
205-
if not param.get("required"):
206-
continue
207-
name = param.get("name")
208-
location = param.get("in")
209-
if isinstance(name, str) and isinstance(location, str):
210-
required.add((name, location))
211-
return required
212-
213-
214-
def _resolve_ref(schema: dict, spec: dict, *, max_depth: int = 50) -> dict:
215-
current = schema
216-
seen: set[str] = set()
217-
depth = 0
218-
219-
while isinstance(current, dict) and "$ref" in current:
220-
ref = current["$ref"]
221-
if not isinstance(ref, str) or not ref.startswith("#/"):
222-
return current
223-
if ref in seen or depth >= max_depth:
224-
return current
225-
226-
seen.add(ref)
227-
depth += 1
228-
229-
target: object = spec
230-
for part in ref.removeprefix("#/").split("/"):
231-
if not isinstance(target, dict) or part not in target:
232-
return current
233-
target = target[part]
234-
if not isinstance(target, dict):
235-
return current
236-
current = target
237-
238-
return current
239-
240-
241-
def _required_json_fields(operation: dict, spec: dict) -> set[str]:
242-
request_body = operation.get("requestBody") or {}
243-
if not isinstance(request_body, dict):
244-
return set()
162+
def _run_oasdiff_breakage_check(
163+
prev_spec: Path, cur_spec: Path
164+
) -> tuple[list[dict], int]:
165+
"""Run oasdiff breaking check between two OpenAPI specs.
245166
246-
content = request_body.get("content") or {}
247-
if not isinstance(content, dict):
248-
return set()
249-
250-
json_content = content.get("application/json")
251-
if not isinstance(json_content, dict):
252-
return set()
253-
254-
schema = json_content.get("schema")
255-
if not isinstance(schema, dict):
256-
return set()
257-
258-
return _required_json_fields_from_schema(schema, spec)
259-
260-
261-
def _required_json_fields_from_schema(schema: dict, spec: dict) -> set[str]:
262-
resolved = _resolve_ref(schema, spec)
263-
264-
if "allOf" in resolved and isinstance(resolved["allOf"], list):
265-
required: set[str] = set()
266-
for item in resolved["allOf"]:
267-
if isinstance(item, dict):
268-
required |= _required_json_fields_from_schema(item, spec)
269-
return required
270-
271-
if resolved.get("type") != "object":
272-
return set()
273-
274-
required = resolved.get("required")
275-
if not isinstance(required, list):
276-
return set()
277-
278-
return {field for field in required if isinstance(field, str)}
167+
Returns (list of breaking changes, exit code from oasdiff).
168+
"""
169+
try:
170+
result = subprocess.run(
171+
[
172+
"oasdiff",
173+
"breaking",
174+
"-f",
175+
"json",
176+
"--fail-on",
177+
"ERR",
178+
str(prev_spec),
179+
str(cur_spec),
180+
],
181+
capture_output=True,
182+
text=True,
183+
)
184+
except FileNotFoundError:
185+
print(
186+
"::warning title=oasdiff not found::"
187+
"Please install oasdiff: https://github.com/oasdiff/oasdiff"
188+
)
189+
return [], 0
279190

191+
breaking_changes = []
192+
if result.stdout:
193+
try:
194+
breaking_changes = json.loads(result.stdout)
195+
except json.JSONDecodeError:
196+
pass
280197

281-
def _is_request_body_required(operation: dict) -> bool:
282-
request_body = operation.get("requestBody")
283-
if not isinstance(request_body, dict):
284-
return False
285-
return bool(request_body.get("required"))
198+
return breaking_changes, result.returncode
286199

287200

288201
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:
293206
return (cur.major, cur.minor) != (prev.major, prev.minor)
294207

295208

296-
def _compute_breakages(
297-
prev_schema: dict, current_schema: dict
298-
) -> tuple[list[str], list[OperationKey]]:
299-
prev_ops = dict(_iter_operations(prev_schema))
300-
cur_ops = dict(_iter_operations(current_schema))
301-
302-
removed = set(prev_ops).difference(cur_ops)
303-
304-
undeprecated_removals: list[OperationKey] = []
305-
for key in sorted(removed, key=lambda k: (k.path, k.method)):
306-
if not prev_ops[key].get("deprecated"):
307-
undeprecated_removals.append(key)
308-
309-
breaking_reasons: list[str] = []
310-
311-
if removed:
312-
breaking_reasons.append(f"Removed operations: {len(removed)}")
313-
314-
for key, prev_op in prev_ops.items():
315-
cur_op = cur_ops.get(key)
316-
if cur_op is None:
317-
continue
318-
319-
new_required_params = _required_parameters(cur_op) - _required_parameters(
320-
prev_op
321-
)
322-
if new_required_params:
323-
formatted = ", ".join(
324-
sorted(f"{n}({loc})" for n, loc in new_required_params)
325-
)
326-
breaking_reasons.append(
327-
f"{key.method.upper()} {key.path}: new required params: {formatted}"
328-
)
329-
330-
if _is_request_body_required(cur_op) and not _is_request_body_required(prev_op):
331-
breaking_reasons.append(
332-
f"{key.method.upper()} {key.path}: request body became required"
333-
)
334-
335-
prev_required_fields = _required_json_fields(prev_op, prev_schema)
336-
cur_required_fields = _required_json_fields(cur_op, current_schema)
337-
new_required_fields = cur_required_fields - prev_required_fields
338-
if new_required_fields:
339-
formatted = ", ".join(sorted(new_required_fields))
340-
breaking_reasons.append(
341-
f"{key.method.upper()} {key.path}: "
342-
f"new required JSON fields: {formatted}"
343-
)
344-
345-
return breaking_reasons, undeprecated_removals
346-
347-
348209
def main() -> int:
349210
current_version = _read_version_from_pyproject(AGENT_SERVER_PYPROJECT)
350211
prev_version = _get_previous_version(PYPI_DISTRIBUTION, current_version)
@@ -362,34 +223,81 @@ def main() -> int:
362223

363224
current_schema = _generate_current_openapi()
364225

365-
breaking_reasons, undeprecated_removals = _compute_breakages(
366-
prev_schema, current_schema
367-
)
226+
with tempfile.TemporaryDirectory(prefix="oasdiff-specs-") as tmp:
227+
tmp_path = Path(tmp)
228+
prev_spec_file = tmp_path / "prev_spec.json"
229+
cur_spec_file = tmp_path / "cur_spec.json"
230+
231+
prev_spec_file.write_text(json.dumps(prev_schema, indent=2))
232+
cur_spec_file.write_text(json.dumps(current_schema, indent=2))
233+
234+
breaking_changes, exit_code = _run_oasdiff_breakage_check(
235+
prev_spec_file, cur_spec_file
236+
)
237+
238+
if not breaking_changes:
239+
if exit_code == 0:
240+
print("No breaking changes detected.")
241+
else:
242+
print(
243+
f"oasdiff returned exit code {exit_code} but no breaking changes "
244+
"in JSON format. There may be warnings only."
245+
)
246+
return 0
247+
248+
removed_operations = []
249+
other_breakage = []
250+
251+
for change in breaking_changes:
252+
change_id = change.get("id", "")
253+
text = change.get("text", "")
254+
details = change.get("details", {})
255+
256+
if "removed" in change_id.lower() and "operation" in change_id.lower():
257+
path = details.get("path", "")
258+
method = details.get("method", "")
259+
operation_id = details.get("operationId", "")
260+
deprecated = details.get("deprecated", False)
261+
262+
removed_operations.append(
263+
{
264+
"path": path,
265+
"method": method,
266+
"operationId": operation_id,
267+
"deprecated": deprecated,
268+
}
269+
)
270+
else:
271+
other_breakage.append(text)
272+
273+
undeprecated_removals = [
274+
op for op in removed_operations if not op.get("deprecated", False)
275+
]
368276

369277
if undeprecated_removals:
370-
for key in undeprecated_removals:
278+
for op in undeprecated_removals:
371279
print(
372-
"::error "
373-
f"title={PYPI_DISTRIBUTION} REST API::Removed {key.method.upper()} "
374-
f"{key.path} without prior deprecation (deprecated=true)."
280+
f"::error "
281+
f"title={PYPI_DISTRIBUTION} REST API::Removed {op['method'].upper()} "
282+
f"{op['path']} without prior deprecation (deprecated=true)."
375283
)
376284

377-
breaking = bool(breaking_reasons)
285+
has_breaking = bool(breaking_changes)
378286

379-
if breaking and not _is_minor_or_major_bump(current_version, prev_version):
287+
if has_breaking and not _is_minor_or_major_bump(current_version, prev_version):
380288
print(
381289
"::error "
382290
f"title={PYPI_DISTRIBUTION} REST API::Breaking REST API change detected "
383291
f"without MINOR version bump ({prev_version} -> {current_version})."
384292
)
385293

386-
if breaking:
387-
print("Breaking REST API changes detected compared to previous release:")
388-
for reason in breaking_reasons:
389-
print(f"- {reason}")
294+
if has_breaking:
295+
print("\nBreaking REST API changes detected compared to previous release:")
296+
for text in breaking_changes:
297+
print(f"- {text.get('text', str(text))}")
390298

391299
errors = bool(undeprecated_removals) or (
392-
breaking and not _is_minor_or_major_bump(current_version, prev_version)
300+
has_breaking and not _is_minor_or_major_bump(current_version, prev_version)
393301
)
394302
return 1 if errors else 0
395303

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@ jobs:
2626
- name: Install workspace deps (dev)
2727
run: uv sync --frozen --group dev
2828

29+
- name: Install oasdiff
30+
run: |
31+
curl -L https://raw.githubusercontent.com/oasdiff/oasdiff/main/install.sh | sh -s -- -b /usr/local/bin
32+
oasdiff --version
33+
2934
- name: Run agent server REST API breakage check
3035
id: api_breakage
3136
# Release PRs (rel-*) must block on breakage.

openhands-agent-server/AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ Removing an endpoint is a breaking change.
4343
### CI enforcement
4444

4545
The workflow `Agent server REST API breakage checks` compares the current OpenAPI
46-
schema against the previous `openhands-agent-server` release on PyPI.
46+
schema against the previous `openhands-agent-server` release on PyPI using [oasdiff](https://github.com/oasdiff/oasdiff).
4747

4848
It currently enforces:
4949
- No removal of operations (path + method) unless they were already marked

0 commit comments

Comments
 (0)