Skip to content

Commit 7158153

Browse files
Revert "fix(ci): compare REST API changes to PR base ref"
This reverts commit 86dafea.
1 parent 86dafea commit 7158153

File tree

4 files changed

+69
-237
lines changed

4 files changed

+69
-237
lines changed

.github/scripts/check_agent_server_rest_api_breakage.py

Lines changed: 69 additions & 161 deletions
Original file line numberDiff line numberDiff line change
@@ -39,21 +39,14 @@
3939
minor releases of runway, so incompatible replacements must ship additively or
4040
behind a versioned contract until the scheduled removal version.
4141
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.
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.
5044
"""
5145

5246
from __future__ import annotations
5347

5448
import ast
5549
import json
56-
import os
5750
import re
5851
import subprocess
5952
import sys
@@ -68,7 +61,6 @@
6861
REPO_ROOT = Path(__file__).resolve().parents[2]
6962
AGENT_SERVER_PYPROJECT = REPO_ROOT / "openhands-agent-server" / "pyproject.toml"
7063
PYPI_DISTRIBUTION = "openhands-agent-server"
71-
BASE_REF_ENV = "REST_API_BREAKAGE_BASE_REF"
7264
# Keep this in sync with REST_ROUTE_DEPRECATION_RE in check_deprecations.py so
7365
# the REST breakage and deprecation checks recognize the same wording.
7466
REST_ROUTE_DEPRECATION_RE = re.compile(
@@ -209,34 +201,6 @@ def _generate_openapi_for_git_ref(git_ref: str) -> dict | None:
209201
return _generate_openapi_from_source_tree(source_tree, git_ref)
210202

211203

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-
240204
def _dotted_name(node: ast.AST) -> str | None:
241205
if isinstance(node, ast.Name):
242206
return node.id
@@ -587,19 +551,37 @@ def _run_oasdiff_breakage_check(
587551
return breaking_changes, result.returncode
588552

589553

590-
def _normalized_openapi_copy(schema: dict) -> dict:
591-
return _normalize_openapi_for_oasdiff(json.loads(json.dumps(schema)))
554+
def main() -> int:
555+
current_version = _read_version_from_pyproject(AGENT_SERVER_PYPROJECT)
556+
baseline_version = _get_baseline_version(PYPI_DISTRIBUTION, current_version)
592557

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
593564

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)
565+
baseline_git_ref = f"v{baseline_version}"
566+
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)
603585

604586
with tempfile.TemporaryDirectory(prefix="oasdiff-specs-") as tmp:
605587
tmp_path = Path(tmp)
@@ -615,136 +597,62 @@ def _check_breaking_changes(
615597

616598
if not breaking_changes:
617599
if exit_code == 0:
618-
print(f"No breaking changes detected against {comparison_label}.")
600+
print("No breaking changes detected.")
619601
else:
620602
print(
621603
f"oasdiff returned exit code {exit_code} but no breaking changes "
622-
f"in JSON format while checking {comparison_label}. There may be "
623-
"warnings only."
604+
"in JSON format. There may be warnings only."
624605
)
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(
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(
633613
removed_operations,
634614
prev_schema,
635615
current_version,
636616
)
637-
]
638-
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-
)
657-
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
670617

618+
for error in removal_errors:
619+
print(f"::error title={PYPI_DISTRIBUTION} REST API::{error}")
671620

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] = []
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))}")
688630

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:
631+
if other_breaking_changes:
693632
print(
694-
f"::warning title={PYPI_DISTRIBUTION} REST API::Unable to resolve "
695-
f"base ref {base_ref!r}; skipping PR-base OpenAPI comparison."
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."
696640
)
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-
)
714641

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:
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):
725647
print(
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."
648+
"Breaking changes are limited to previously-deprecated operations "
649+
"whose scheduled removal versions have been reached, and/or "
650+
"additive response oneOf expansions."
729651
)
730652
else:
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}")
653+
return 1
742654

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

749657

750658
if __name__ == "__main__":

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,6 @@ 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 || '' }}
4240
run: |
4341
uv run --with packaging python .github/scripts/check_agent_server_rest_api_breakage.py 2>&1 | tee api-breakage.log
4442
exit_code=${PIPESTATUS[0]}

AGENTS.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,6 @@ 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`.
115114

116115

117116
## Package-specific guidance

tests/ci_scripts/test_check_agent_server_rest_api_breakage.py

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

55
import importlib.util
6-
import json
76
import sys
87
from pathlib import Path
98

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

377376

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-
450377
def test_main_rejects_non_removal_breakage_even_with_newer_version(monkeypatch, capsys):
451378
monkeypatch.setattr(_prod, "_read_version_from_pyproject", lambda _path: "1.15.0")
452379
monkeypatch.setattr(

0 commit comments

Comments
 (0)