Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
230 changes: 161 additions & 69 deletions .github/scripts/check_agent_server_rest_api_breakage.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,21 @@
minor releases of runway, so incompatible replacements must ship additively or
behind a versioned contract until the scheduled removal version.

If the baseline release schema can't be generated (e.g., missing tag / repo issues),
the script emits a warning and exits successfully to avoid flaky CI.
5) Pull requests are also checked against their base branch when available
- This catches unreleased endpoint removals or incompatible contract edits that
may not appear in the latest published baseline yet.
- The release-baseline comparison still runs so published compatibility policy
remains enforced.

If a comparison schema can't be generated (e.g., missing tag / repo issues),
the script emits a warning and continues with the remaining checks to avoid flaky CI.
"""

from __future__ import annotations

import ast
import json
import os
import re
import subprocess
import sys
Expand All @@ -61,6 +68,7 @@
REPO_ROOT = Path(__file__).resolve().parents[2]
AGENT_SERVER_PYPROJECT = REPO_ROOT / "openhands-agent-server" / "pyproject.toml"
PYPI_DISTRIBUTION = "openhands-agent-server"
BASE_REF_ENV = "REST_API_BREAKAGE_BASE_REF"
# Keep this in sync with REST_ROUTE_DEPRECATION_RE in check_deprecations.py so
# the REST breakage and deprecation checks recognize the same wording.
REST_ROUTE_DEPRECATION_RE = re.compile(
Expand Down Expand Up @@ -201,6 +209,34 @@ def _generate_openapi_for_git_ref(git_ref: str) -> dict | None:
return _generate_openapi_from_source_tree(source_tree, git_ref)


def _get_base_ref() -> str | None:
base_ref = (
os.environ.get(BASE_REF_ENV) or os.environ.get("GITHUB_BASE_REF") or ""
).strip()
return base_ref or None


def _resolve_git_ref(ref: str) -> str | None:
for candidate in (f"origin/{ref}", ref):
result = subprocess.run(
[
"git",
"-C",
str(REPO_ROOT),
"rev-parse",
"--verify",
"--quiet",
candidate,
],
check=False,
capture_output=True,
text=True,
)
if result.returncode == 0:
return candidate
return None


def _dotted_name(node: ast.AST) -> str | None:
if isinstance(node, ast.Name):
return node.id
Expand Down Expand Up @@ -551,37 +587,19 @@ def _run_oasdiff_breakage_check(
return breaking_changes, result.returncode


def main() -> int:
current_version = _read_version_from_pyproject(AGENT_SERVER_PYPROJECT)
baseline_version = _get_baseline_version(PYPI_DISTRIBUTION, current_version)

if baseline_version is None:
print(
f"::warning title={PYPI_DISTRIBUTION} REST API::Unable to find baseline "
f"version for {current_version}; skipping breakage checks."
)
return 0
def _normalized_openapi_copy(schema: dict) -> dict:
return _normalize_openapi_for_oasdiff(json.loads(json.dumps(schema)))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Using json.loads(json.dumps(schema)) for deep copying is inefficient and could lose Python-specific types. Consider using copy.deepcopy(schema) instead:

Suggested change
return _normalize_openapi_for_oasdiff(json.loads(json.dumps(schema)))
return _normalize_openapi_for_oasdiff(copy.deepcopy(schema))

You'd need to add import copy at the top. Not blocking since schemas are likely small, but it's cleaner and more explicit.


baseline_git_ref = f"v{baseline_version}"

static_policy_errors = _find_sdk_deprecated_fastapi_routes(REPO_ROOT)
for error in static_policy_errors:
print(f"::error title={PYPI_DISTRIBUTION} REST API::{error}")

current_schema = _generate_current_openapi()
if current_schema is None:
return 1

deprecation_policy_errors = _find_deprecation_policy_errors(current_schema)
for error in deprecation_policy_errors:
print(f"::error title={PYPI_DISTRIBUTION} REST API::{error}")

prev_schema = _generate_openapi_for_git_ref(baseline_git_ref)
if prev_schema is None:
return 0 if not (static_policy_errors or deprecation_policy_errors) else 1

prev_schema = _normalize_openapi_for_oasdiff(prev_schema)
current_schema = _normalize_openapi_for_oasdiff(current_schema)
def _check_breaking_changes(
*,
prev_schema: dict,
current_schema: dict,
current_version: str,
comparison_label: str,
) -> list[str]:
prev_schema = _normalized_openapi_copy(prev_schema)
current_schema = _normalized_openapi_copy(current_schema)

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

if not breaking_changes:
if exit_code == 0:
print("No breaking changes detected.")
print(f"No breaking changes detected against {comparison_label}.")
else:
print(
f"oasdiff returned exit code {exit_code} but no breaking changes "
"in JSON format. There may be warnings only."
f"in JSON format while checking {comparison_label}. There may be "
"warnings only."
)
else:
(
removed_operations,
additive_response_oneof,
other_breaking_changes,
) = _split_breaking_changes(breaking_changes)
removal_errors = _validate_removed_operations(
return []

removed_operations, additive_response_oneof, other_breaking_changes = (
_split_breaking_changes(breaking_changes)
)
errors = [
f"{comparison_label}: {error}"
for error in _validate_removed_operations(
removed_operations,
prev_schema,
current_version,
)
]

for error in removal_errors:
print(f"::error title={PYPI_DISTRIBUTION} REST API::{error}")
if additive_response_oneof:
print(
f"\n::notice title={PYPI_DISTRIBUTION} REST API::"
f"Additive oneOf/anyOf expansion detected in response schemas against "
f"{comparison_label}. This is expected for extensible "
"discriminated-union APIs and does not break backward compatibility."
)
for item in additive_response_oneof:
print(f" - {item.get('text', str(item))}")

if other_breaking_changes:
errors.append(
f"{comparison_label}: Detected breaking REST API changes other than "
"removing previously-deprecated operations or additive response "
"oneOf expansions. REST contract changes must preserve compatibility "
"for 5 minor releases; keep the old contract available until its "
"scheduled removal version."
)

if additive_response_oneof:
print(
f"\n::notice title={PYPI_DISTRIBUTION} REST API::"
"Additive oneOf/anyOf expansion detected in response schemas. "
"This is expected for extensible discriminated-union APIs and "
"does not break backward compatibility."
)
for item in additive_response_oneof:
print(f" - {item.get('text', str(item))}")
print(f"\nBreaking REST API changes detected against {comparison_label}:")
for text in breaking_changes:
print(f"- {text.get('text', str(text))}")

if not errors:
print(
"Breaking changes are limited to previously-deprecated operations "
"whose scheduled removal versions have been reached, and/or additive "
"response oneOf expansions."
)

return errors

if other_breaking_changes:

def main() -> int:
current_version = _read_version_from_pyproject(AGENT_SERVER_PYPROJECT)

static_policy_errors = _find_sdk_deprecated_fastapi_routes(REPO_ROOT)
for error in static_policy_errors:
print(f"::error title={PYPI_DISTRIBUTION} REST API::{error}")

current_schema = _generate_current_openapi()
if current_schema is None:
return 1

deprecation_policy_errors = _find_deprecation_policy_errors(current_schema)
for error in deprecation_policy_errors:
print(f"::error title={PYPI_DISTRIBUTION} REST API::{error}")

comparison_errors: list[str] = []

base_ref = _get_base_ref()
if base_ref is not None:
resolved_base_ref = _resolve_git_ref(base_ref)
if resolved_base_ref is None:
print(
"::error "
f"title={PYPI_DISTRIBUTION} REST API::Detected breaking REST API "
"changes other than removing previously-deprecated operations "
"or additive response oneOf expansions. "
"REST contract changes must preserve compatibility for 5 minor "
"releases; keep the old contract available until its scheduled "
"removal version."
f"::warning title={PYPI_DISTRIBUTION} REST API::Unable to resolve "
f"base ref {base_ref!r}; skipping PR-base OpenAPI comparison."
)
else:
base_schema = _generate_openapi_for_git_ref(resolved_base_ref)
if base_schema is None:
print(
f"::warning title={PYPI_DISTRIBUTION} REST API::Unable to "
f"generate OpenAPI schema for base ref {resolved_base_ref}; "
"skipping PR-base comparison."
)
else:
comparison_errors.extend(
_check_breaking_changes(
prev_schema=base_schema,
current_schema=current_schema,
current_version=current_version,
comparison_label=f"PR base ref {resolved_base_ref}",
)
)

print("\nBreaking REST API changes detected compared to baseline release:")
for text in breaking_changes:
print(f"- {text.get('text', str(text))}")

if not (removal_errors or other_breaking_changes):
baseline_version = _get_baseline_version(PYPI_DISTRIBUTION, current_version)
if baseline_version is None:
print(
f"::warning title={PYPI_DISTRIBUTION} REST API::Unable to find baseline "
f"version for {current_version}; skipping release-baseline checks."
)
else:
baseline_git_ref = f"v{baseline_version}"
prev_schema = _generate_openapi_for_git_ref(baseline_git_ref)
if prev_schema is None:
print(
"Breaking changes are limited to previously-deprecated operations "
"whose scheduled removal versions have been reached, and/or "
"additive response oneOf expansions."
f"::warning title={PYPI_DISTRIBUTION} REST API::Unable to generate "
f"OpenAPI schema for baseline {baseline_git_ref}; skipping "
"release-baseline comparison."
)
else:
return 1
comparison_errors.extend(
_check_breaking_changes(
prev_schema=prev_schema,
current_schema=current_schema,
current_version=current_version,
comparison_label=f"baseline release {baseline_git_ref}",
)
)

for error in comparison_errors:
print(f"::error title={PYPI_DISTRIBUTION} REST API::{error}")

return 1 if (static_policy_errors or deprecation_policy_errors) else 0
return (
1
if (static_policy_errors or deprecation_policy_errors or comparison_errors)
else 0
)


if __name__ == "__main__":
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/agent-server-rest-api-breakage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ jobs:
id: api_breakage
# Let this step fail so CI is visibly red on breakage.
# Later reporting steps still run because they use if: always().
env:
REST_API_BREAKAGE_BASE_REF: ${{ github.event_name == 'pull_request' && github.base_ref || '' }}
run: |
uv run --with packaging python .github/scripts/check_agent_server_rest_api_breakage.py 2>&1 | tee api-breakage.log
exit_code=${PIPESTATUS[0]}
Expand Down
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ When reviewing code, provide constructive feedback:
- 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.
- `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.
- 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.
- 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`.


## Package-specific guidance
Expand Down
73 changes: 73 additions & 0 deletions tests/ci_scripts/test_check_agent_server_rest_api_breakage.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from __future__ import annotations

import importlib.util
import json
import sys
from pathlib import Path

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


def test_main_rejects_breakage_against_pr_base_ref(monkeypatch, capsys):
monkeypatch.setenv(_prod.BASE_REF_ENV, "main")
monkeypatch.setattr(_prod, "_read_version_from_pyproject", lambda _path: "1.16.1")
monkeypatch.setattr(
_prod, "_get_baseline_version", lambda _distribution, _current: "1.16.1"
)
monkeypatch.setattr(_prod, "_resolve_git_ref", lambda ref: f"origin/{ref}")
monkeypatch.setattr(_prod, "_find_sdk_deprecated_fastapi_routes", lambda _root: [])
monkeypatch.setattr(_prod, "_find_deprecation_policy_errors", lambda _schema: [])
monkeypatch.setattr(_prod, "_generate_current_openapi", lambda: {"paths": {}})

base_schema = _schema_with_operation("/foo", "get", {"responses": {}})
monkeypatch.setattr(
_prod,
"_generate_openapi_for_git_ref",
lambda ref: base_schema if ref == "origin/main" else {"paths": {}},
)

def _fake_run(prev_spec, _cur_spec):
prev_schema = json.loads(Path(prev_spec).read_text())
if "/foo" in prev_schema.get("paths", {}):
return (
[
{
"id": "removed-operation",
"details": {
"path": "/foo",
"method": "get",
"deprecated": False,
},
"text": "removed GET /foo",
}
],
1,
)
return [], 0

monkeypatch.setattr(_prod, "_run_oasdiff_breakage_check", _fake_run)

assert _prod.main() == 1

captured = capsys.readouterr()
assert "PR base ref origin/main" in captured.out
assert "Removed GET /foo without prior deprecation" in captured.out


def test_main_warns_when_pr_base_ref_cannot_be_resolved(monkeypatch, capsys):
monkeypatch.setenv(_prod.BASE_REF_ENV, "main")
monkeypatch.setattr(_prod, "_read_version_from_pyproject", lambda _path: "1.16.1")
monkeypatch.setattr(
_prod, "_get_baseline_version", lambda _distribution, _current: "1.16.1"
)
monkeypatch.setattr(_prod, "_resolve_git_ref", lambda _ref: None)
monkeypatch.setattr(_prod, "_find_sdk_deprecated_fastapi_routes", lambda _root: [])
monkeypatch.setattr(_prod, "_find_deprecation_policy_errors", lambda _schema: [])
monkeypatch.setattr(_prod, "_generate_current_openapi", lambda: {"paths": {}})
monkeypatch.setattr(
_prod, "_generate_openapi_for_git_ref", lambda _ref: {"paths": {}}
)
monkeypatch.setattr(
_prod, "_run_oasdiff_breakage_check", lambda _prev, _cur: ([], 0)
)

assert _prod.main() == 0

captured = capsys.readouterr()
assert "Unable to resolve base ref 'main'" in captured.out
assert (
"No breaking changes detected against baseline release v1.16.1." in captured.out
)


def test_main_rejects_non_removal_breakage_even_with_newer_version(monkeypatch, capsys):
monkeypatch.setattr(_prod, "_read_version_from_pyproject", lambda _path: "1.15.0")
monkeypatch.setattr(
Expand Down
Loading