Skip to content

Commit 86dafea

Browse files
fix(ci): compare REST API changes to PR base ref
Add a PR-base OpenAPI comparison to the REST API breakage check so unreleased endpoint removals on main are caught before merge. Co-authored-by: openhands <openhands@all-hands.dev>
1 parent 56cd2ab commit 86dafea

File tree

4 files changed

+237
-69
lines changed

4 files changed

+237
-69
lines changed

.github/scripts/check_agent_server_rest_api_breakage.py

Lines changed: 161 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,21 @@
3939
minor releases of runway, so incompatible replacements must ship additively or
4040
behind a versioned contract until the scheduled removal version.
4141
42-
If the baseline release schema can't be generated (e.g., missing tag / repo issues),
43-
the script emits a warning and exits successfully to avoid flaky CI.
42+
5) Pull requests are also checked against their base branch when available
43+
- This catches unreleased endpoint removals or incompatible contract edits that
44+
may not appear in the latest published baseline yet.
45+
- The release-baseline comparison still runs so published compatibility policy
46+
remains enforced.
47+
48+
If a comparison schema can't be generated (e.g., missing tag / repo issues),
49+
the script emits a warning and continues with the remaining checks to avoid flaky CI.
4450
"""
4551

4652
from __future__ import annotations
4753

4854
import ast
4955
import json
56+
import os
5057
import re
5158
import subprocess
5259
import sys
@@ -61,6 +68,7 @@
6168
REPO_ROOT = Path(__file__).resolve().parents[2]
6269
AGENT_SERVER_PYPROJECT = REPO_ROOT / "openhands-agent-server" / "pyproject.toml"
6370
PYPI_DISTRIBUTION = "openhands-agent-server"
71+
BASE_REF_ENV = "REST_API_BREAKAGE_BASE_REF"
6472
# Keep this in sync with REST_ROUTE_DEPRECATION_RE in check_deprecations.py so
6573
# the REST breakage and deprecation checks recognize the same wording.
6674
REST_ROUTE_DEPRECATION_RE = re.compile(
@@ -201,6 +209,34 @@ def _generate_openapi_for_git_ref(git_ref: str) -> dict | None:
201209
return _generate_openapi_from_source_tree(source_tree, git_ref)
202210

203211

212+
def _get_base_ref() -> str | None:
213+
base_ref = (
214+
os.environ.get(BASE_REF_ENV) or os.environ.get("GITHUB_BASE_REF") or ""
215+
).strip()
216+
return base_ref or None
217+
218+
219+
def _resolve_git_ref(ref: str) -> str | None:
220+
for candidate in (f"origin/{ref}", ref):
221+
result = subprocess.run(
222+
[
223+
"git",
224+
"-C",
225+
str(REPO_ROOT),
226+
"rev-parse",
227+
"--verify",
228+
"--quiet",
229+
candidate,
230+
],
231+
check=False,
232+
capture_output=True,
233+
text=True,
234+
)
235+
if result.returncode == 0:
236+
return candidate
237+
return None
238+
239+
204240
def _dotted_name(node: ast.AST) -> str | None:
205241
if isinstance(node, ast.Name):
206242
return node.id
@@ -551,37 +587,19 @@ def _run_oasdiff_breakage_check(
551587
return breaking_changes, result.returncode
552588

553589

554-
def main() -> int:
555-
current_version = _read_version_from_pyproject(AGENT_SERVER_PYPROJECT)
556-
baseline_version = _get_baseline_version(PYPI_DISTRIBUTION, current_version)
557-
558-
if baseline_version is None:
559-
print(
560-
f"::warning title={PYPI_DISTRIBUTION} REST API::Unable to find baseline "
561-
f"version for {current_version}; skipping breakage checks."
562-
)
563-
return 0
590+
def _normalized_openapi_copy(schema: dict) -> dict:
591+
return _normalize_openapi_for_oasdiff(json.loads(json.dumps(schema)))
564592

565-
baseline_git_ref = f"v{baseline_version}"
566593

567-
static_policy_errors = _find_sdk_deprecated_fastapi_routes(REPO_ROOT)
568-
for error in static_policy_errors:
569-
print(f"::error title={PYPI_DISTRIBUTION} REST API::{error}")
570-
571-
current_schema = _generate_current_openapi()
572-
if current_schema is None:
573-
return 1
574-
575-
deprecation_policy_errors = _find_deprecation_policy_errors(current_schema)
576-
for error in deprecation_policy_errors:
577-
print(f"::error title={PYPI_DISTRIBUTION} REST API::{error}")
578-
579-
prev_schema = _generate_openapi_for_git_ref(baseline_git_ref)
580-
if prev_schema is None:
581-
return 0 if not (static_policy_errors or deprecation_policy_errors) else 1
582-
583-
prev_schema = _normalize_openapi_for_oasdiff(prev_schema)
584-
current_schema = _normalize_openapi_for_oasdiff(current_schema)
594+
def _check_breaking_changes(
595+
*,
596+
prev_schema: dict,
597+
current_schema: dict,
598+
current_version: str,
599+
comparison_label: str,
600+
) -> list[str]:
601+
prev_schema = _normalized_openapi_copy(prev_schema)
602+
current_schema = _normalized_openapi_copy(current_schema)
585603

586604
with tempfile.TemporaryDirectory(prefix="oasdiff-specs-") as tmp:
587605
tmp_path = Path(tmp)
@@ -597,62 +615,136 @@ def main() -> int:
597615

598616
if not breaking_changes:
599617
if exit_code == 0:
600-
print("No breaking changes detected.")
618+
print(f"No breaking changes detected against {comparison_label}.")
601619
else:
602620
print(
603621
f"oasdiff returned exit code {exit_code} but no breaking changes "
604-
"in JSON format. There may be warnings only."
622+
f"in JSON format while checking {comparison_label}. There may be "
623+
"warnings only."
605624
)
606-
else:
607-
(
608-
removed_operations,
609-
additive_response_oneof,
610-
other_breaking_changes,
611-
) = _split_breaking_changes(breaking_changes)
612-
removal_errors = _validate_removed_operations(
625+
return []
626+
627+
removed_operations, additive_response_oneof, other_breaking_changes = (
628+
_split_breaking_changes(breaking_changes)
629+
)
630+
errors = [
631+
f"{comparison_label}: {error}"
632+
for error in _validate_removed_operations(
613633
removed_operations,
614634
prev_schema,
615635
current_version,
616636
)
637+
]
617638

618-
for error in removal_errors:
619-
print(f"::error title={PYPI_DISTRIBUTION} REST API::{error}")
639+
if additive_response_oneof:
640+
print(
641+
f"\n::notice title={PYPI_DISTRIBUTION} REST API::"
642+
f"Additive oneOf/anyOf expansion detected in response schemas against "
643+
f"{comparison_label}. This is expected for extensible "
644+
"discriminated-union APIs and does not break backward compatibility."
645+
)
646+
for item in additive_response_oneof:
647+
print(f" - {item.get('text', str(item))}")
648+
649+
if other_breaking_changes:
650+
errors.append(
651+
f"{comparison_label}: Detected breaking REST API changes other than "
652+
"removing previously-deprecated operations or additive response "
653+
"oneOf expansions. REST contract changes must preserve compatibility "
654+
"for 5 minor releases; keep the old contract available until its "
655+
"scheduled removal version."
656+
)
620657

621-
if additive_response_oneof:
622-
print(
623-
f"\n::notice title={PYPI_DISTRIBUTION} REST API::"
624-
"Additive oneOf/anyOf expansion detected in response schemas. "
625-
"This is expected for extensible discriminated-union APIs and "
626-
"does not break backward compatibility."
627-
)
628-
for item in additive_response_oneof:
629-
print(f" - {item.get('text', str(item))}")
658+
print(f"\nBreaking REST API changes detected against {comparison_label}:")
659+
for text in breaking_changes:
660+
print(f"- {text.get('text', str(text))}")
661+
662+
if not errors:
663+
print(
664+
"Breaking changes are limited to previously-deprecated operations "
665+
"whose scheduled removal versions have been reached, and/or additive "
666+
"response oneOf expansions."
667+
)
668+
669+
return errors
630670

631-
if other_breaking_changes:
671+
672+
def main() -> int:
673+
current_version = _read_version_from_pyproject(AGENT_SERVER_PYPROJECT)
674+
675+
static_policy_errors = _find_sdk_deprecated_fastapi_routes(REPO_ROOT)
676+
for error in static_policy_errors:
677+
print(f"::error title={PYPI_DISTRIBUTION} REST API::{error}")
678+
679+
current_schema = _generate_current_openapi()
680+
if current_schema is None:
681+
return 1
682+
683+
deprecation_policy_errors = _find_deprecation_policy_errors(current_schema)
684+
for error in deprecation_policy_errors:
685+
print(f"::error title={PYPI_DISTRIBUTION} REST API::{error}")
686+
687+
comparison_errors: list[str] = []
688+
689+
base_ref = _get_base_ref()
690+
if base_ref is not None:
691+
resolved_base_ref = _resolve_git_ref(base_ref)
692+
if resolved_base_ref is None:
632693
print(
633-
"::error "
634-
f"title={PYPI_DISTRIBUTION} REST API::Detected breaking REST API "
635-
"changes other than removing previously-deprecated operations "
636-
"or additive response oneOf expansions. "
637-
"REST contract changes must preserve compatibility for 5 minor "
638-
"releases; keep the old contract available until its scheduled "
639-
"removal version."
694+
f"::warning title={PYPI_DISTRIBUTION} REST API::Unable to resolve "
695+
f"base ref {base_ref!r}; skipping PR-base OpenAPI comparison."
640696
)
697+
else:
698+
base_schema = _generate_openapi_for_git_ref(resolved_base_ref)
699+
if base_schema is None:
700+
print(
701+
f"::warning title={PYPI_DISTRIBUTION} REST API::Unable to "
702+
f"generate OpenAPI schema for base ref {resolved_base_ref}; "
703+
"skipping PR-base comparison."
704+
)
705+
else:
706+
comparison_errors.extend(
707+
_check_breaking_changes(
708+
prev_schema=base_schema,
709+
current_schema=current_schema,
710+
current_version=current_version,
711+
comparison_label=f"PR base ref {resolved_base_ref}",
712+
)
713+
)
641714

642-
print("\nBreaking REST API changes detected compared to baseline release:")
643-
for text in breaking_changes:
644-
print(f"- {text.get('text', str(text))}")
645-
646-
if not (removal_errors or other_breaking_changes):
715+
baseline_version = _get_baseline_version(PYPI_DISTRIBUTION, current_version)
716+
if baseline_version is None:
717+
print(
718+
f"::warning title={PYPI_DISTRIBUTION} REST API::Unable to find baseline "
719+
f"version for {current_version}; skipping release-baseline checks."
720+
)
721+
else:
722+
baseline_git_ref = f"v{baseline_version}"
723+
prev_schema = _generate_openapi_for_git_ref(baseline_git_ref)
724+
if prev_schema is None:
647725
print(
648-
"Breaking changes are limited to previously-deprecated operations "
649-
"whose scheduled removal versions have been reached, and/or "
650-
"additive response oneOf expansions."
726+
f"::warning title={PYPI_DISTRIBUTION} REST API::Unable to generate "
727+
f"OpenAPI schema for baseline {baseline_git_ref}; skipping "
728+
"release-baseline comparison."
651729
)
652730
else:
653-
return 1
731+
comparison_errors.extend(
732+
_check_breaking_changes(
733+
prev_schema=prev_schema,
734+
current_schema=current_schema,
735+
current_version=current_version,
736+
comparison_label=f"baseline release {baseline_git_ref}",
737+
)
738+
)
739+
740+
for error in comparison_errors:
741+
print(f"::error title={PYPI_DISTRIBUTION} REST API::{error}")
654742

655-
return 1 if (static_policy_errors or deprecation_policy_errors) else 0
743+
return (
744+
1
745+
if (static_policy_errors or deprecation_policy_errors or comparison_errors)
746+
else 0
747+
)
656748

657749

658750
if __name__ == "__main__":

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ jobs:
3737
id: api_breakage
3838
# Let this step fail so CI is visibly red on breakage.
3939
# Later reporting steps still run because they use if: always().
40+
env:
41+
REST_API_BREAKAGE_BASE_REF: ${{ github.event_name == 'pull_request' && github.base_ref || '' }}
4042
run: |
4143
uv run --with packaging python .github/scripts/check_agent_server_rest_api_breakage.py 2>&1 | tee api-breakage.log
4244
exit_code=${PIPESTATUS[0]}

AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ When reviewing code, provide constructive feedback:
111111
- Workspace-wide uv resolver guardrails belong in the repository root `[tool.uv]` table. When `exclude-newer` is configured there, `uv lock` persists it into the root `uv.lock` `[options]` section as both an absolute cutoff and `exclude-newer-span`, and `uv sync --frozen` continues to use that locked workspace state.
112112
- `pr-review-by-openhands` delegates to `OpenHands/extensions/plugins/pr-review@main`. Repo-specific reviewer instructions live in `.agents/skills/custom-codereview-guide.md`, and because task-trigger matching is substring-based, that `/codereview` skill is also auto-injected for the workflow's `/codereview-roasted` prompt.
113113
- Auto-title generation should not re-read `ConversationState.events` from a background task triggered by a freshly received `MessageEvent`; extract message text synchronously from the incoming event and then reuse shared title helpers (`extract_message_text`, `generate_title_from_message`) to avoid persistence-order races.
114+
- The agent-server REST API breakage workflow now does two comparisons when a PR base ref is available: it still checks the latest published baseline from PyPI for release compatibility, and it also diffs the current OpenAPI schema against the PR base branch to catch unreleased endpoint removals or incompatible contract edits already present on `main`.
114115

115116

116117
## Package-specific guidance

tests/ci_scripts/test_check_agent_server_rest_api_breakage.py

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from __future__ import annotations
44

55
import importlib.util
6+
import json
67
import sys
78
from pathlib import Path
89

@@ -374,6 +375,78 @@ def test_main_allows_scheduled_removal_when_baseline_matches_current(
374375
assert "scheduled removal versions have been reached" in captured.out
375376

376377

378+
def test_main_rejects_breakage_against_pr_base_ref(monkeypatch, capsys):
379+
monkeypatch.setenv(_prod.BASE_REF_ENV, "main")
380+
monkeypatch.setattr(_prod, "_read_version_from_pyproject", lambda _path: "1.16.1")
381+
monkeypatch.setattr(
382+
_prod, "_get_baseline_version", lambda _distribution, _current: "1.16.1"
383+
)
384+
monkeypatch.setattr(_prod, "_resolve_git_ref", lambda ref: f"origin/{ref}")
385+
monkeypatch.setattr(_prod, "_find_sdk_deprecated_fastapi_routes", lambda _root: [])
386+
monkeypatch.setattr(_prod, "_find_deprecation_policy_errors", lambda _schema: [])
387+
monkeypatch.setattr(_prod, "_generate_current_openapi", lambda: {"paths": {}})
388+
389+
base_schema = _schema_with_operation("/foo", "get", {"responses": {}})
390+
monkeypatch.setattr(
391+
_prod,
392+
"_generate_openapi_for_git_ref",
393+
lambda ref: base_schema if ref == "origin/main" else {"paths": {}},
394+
)
395+
396+
def _fake_run(prev_spec, _cur_spec):
397+
prev_schema = json.loads(Path(prev_spec).read_text())
398+
if "/foo" in prev_schema.get("paths", {}):
399+
return (
400+
[
401+
{
402+
"id": "removed-operation",
403+
"details": {
404+
"path": "/foo",
405+
"method": "get",
406+
"deprecated": False,
407+
},
408+
"text": "removed GET /foo",
409+
}
410+
],
411+
1,
412+
)
413+
return [], 0
414+
415+
monkeypatch.setattr(_prod, "_run_oasdiff_breakage_check", _fake_run)
416+
417+
assert _prod.main() == 1
418+
419+
captured = capsys.readouterr()
420+
assert "PR base ref origin/main" in captured.out
421+
assert "Removed GET /foo without prior deprecation" in captured.out
422+
423+
424+
def test_main_warns_when_pr_base_ref_cannot_be_resolved(monkeypatch, capsys):
425+
monkeypatch.setenv(_prod.BASE_REF_ENV, "main")
426+
monkeypatch.setattr(_prod, "_read_version_from_pyproject", lambda _path: "1.16.1")
427+
monkeypatch.setattr(
428+
_prod, "_get_baseline_version", lambda _distribution, _current: "1.16.1"
429+
)
430+
monkeypatch.setattr(_prod, "_resolve_git_ref", lambda _ref: None)
431+
monkeypatch.setattr(_prod, "_find_sdk_deprecated_fastapi_routes", lambda _root: [])
432+
monkeypatch.setattr(_prod, "_find_deprecation_policy_errors", lambda _schema: [])
433+
monkeypatch.setattr(_prod, "_generate_current_openapi", lambda: {"paths": {}})
434+
monkeypatch.setattr(
435+
_prod, "_generate_openapi_for_git_ref", lambda _ref: {"paths": {}}
436+
)
437+
monkeypatch.setattr(
438+
_prod, "_run_oasdiff_breakage_check", lambda _prev, _cur: ([], 0)
439+
)
440+
441+
assert _prod.main() == 0
442+
443+
captured = capsys.readouterr()
444+
assert "Unable to resolve base ref 'main'" in captured.out
445+
assert (
446+
"No breaking changes detected against baseline release v1.16.1." in captured.out
447+
)
448+
449+
377450
def test_main_rejects_non_removal_breakage_even_with_newer_version(monkeypatch, capsys):
378451
monkeypatch.setattr(_prod, "_read_version_from_pyproject", lambda _path: "1.15.0")
379452
monkeypatch.setattr(

0 commit comments

Comments
 (0)