Reject SDK @deprecated on FastAPI routes (#2424)

Co-authored-by: openhands <openhands@all-hands.dev>

Author: enyst

.github/scripts/check_agent_server_rest_api_breakage.py

@@ -9,7 +9,8 @@
 
 Policies enforced:
 
-1) Deprecation metadata for REST endpoints
+1) REST deprecations must use FastAPI/OpenAPI metadata
+   - FastAPI route handlers must not use `openhands.sdk.utils.deprecation.deprecated`.
    - Endpoints documented as deprecated in their OpenAPI description must also be
      marked `deprecated: true` in the generated schema.
 
@@ -27,6 +28,7 @@
 
 from __future__ import annotations
 
+import ast
 import json
 import subprocess
 import sys
@@ -51,6 +53,7 @@
     "head",
     "trace",
 }
+ROUTE_DECORATOR_NAMES = HTTP_METHODS | {"api_route"}
 OPENAPI_PROGRAM = """
 import json
 import sys
@@ -173,6 +176,96 @@ def _generate_openapi_for_git_ref(git_ref: str) -> dict | None:
         return _generate_openapi_from_source_tree(source_tree, git_ref)
 
 
+def _dotted_name(node: ast.AST) -> str | None:
+    if isinstance(node, ast.Name):
+        return node.id
+    if isinstance(node, ast.Attribute):
+        prefix = _dotted_name(node.value)
+        if prefix is None:
+            return None
+        return f"{prefix}.{node.attr}"
+    return None
+
+
+def _find_sdk_deprecated_fastapi_routes_in_file(
+    file_path: Path, repo_root: Path
+) -> list[str]:
+    tree = ast.parse(file_path.read_text(), filename=str(file_path))
+
+    deprecated_names: set[str] = set()
+    deprecation_module_names: set[str] = set()
+
+    for node in tree.body:
+        if isinstance(node, ast.ImportFrom):
+            if node.module == "openhands.sdk.utils.deprecation":
+                for alias in node.names:
+                    if alias.name == "deprecated":
+                        deprecated_names.add(alias.asname or alias.name)
+            elif node.module == "openhands.sdk.utils":
+                for alias in node.names:
+                    if alias.name == "deprecation":
+                        deprecation_module_names.add(alias.asname or alias.name)
+        elif isinstance(node, ast.Import):
+            for alias in node.names:
+                if alias.name == "openhands.sdk.utils.deprecation":
+                    deprecation_module_names.add(alias.asname or alias.name)
+
+    errors: list[str] = []
+    for node in ast.walk(tree):
+        if not isinstance(node, ast.FunctionDef | ast.AsyncFunctionDef):
+            continue
+
+        has_route_decorator = False
+        uses_sdk_deprecated = False
+
+        for decorator in node.decorator_list:
+            if not isinstance(decorator, ast.Call):
+                continue
+
+            dotted_name = _dotted_name(decorator.func)
+            if (
+                isinstance(decorator.func, ast.Attribute)
+                and decorator.func.attr in ROUTE_DECORATOR_NAMES
+            ):
+                has_route_decorator = True
+
+            if dotted_name in deprecated_names or (
+                dotted_name == "openhands.sdk.utils.deprecation.deprecated"
+            ):
+                uses_sdk_deprecated = True
+                continue
+
+            if (
+                isinstance(decorator.func, ast.Attribute)
+                and decorator.func.attr == "deprecated"
+            ):
+                base_name = _dotted_name(decorator.func.value)
+                if base_name in deprecation_module_names or (
+                    base_name == "openhands.sdk.utils.deprecation"
+                ):
+                    uses_sdk_deprecated = True
+
+        if has_route_decorator and uses_sdk_deprecated:
+            rel_path = file_path.relative_to(repo_root)
+            errors.append(
+                f"{rel_path}:{node.lineno} FastAPI route `{node.name}` uses "
+                "openhands.sdk.utils.deprecation.deprecated; use the route "
+                "decorator's deprecated=True flag instead."
+            )
+
+    return errors
+
+
+def _find_sdk_deprecated_fastapi_routes(repo_root: Path) -> list[str]:
+    app_root = repo_root / "openhands-agent-server" / "openhands" / "agent_server"
+    errors: list[str] = []
+
+    for file_path in sorted(app_root.rglob("*.py")):
+        errors.extend(_find_sdk_deprecated_fastapi_routes_in_file(file_path, repo_root))
+
+    return errors
+
+
 def _find_deprecation_policy_errors(schema: dict) -> list[str]:
     errors: list[str] = []
 
@@ -301,6 +394,10 @@ def main() -> int:
 
     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
@@ -311,7 +408,7 @@ def main() -> int:
 
     prev_schema = _generate_openapi_for_git_ref(baseline_git_ref)
     if prev_schema is None:
-        return 0 if not deprecation_policy_errors else 1
+        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)
@@ -380,7 +477,7 @@ def main() -> int:
         ):
             return 1
 
-    return 1 if deprecation_policy_errors else 0
+    return 1 if (static_policy_errors or deprecation_policy_errors) else 0
 
 
 if __name__ == "__main__":

openhands-agent-server/AGENTS.md

@@ -28,6 +28,9 @@ When deprecating a REST endpoint:
 2. Add a docstring note that includes:
    - the version it was deprecated in
    - the version it is scheduled for removal in (default: **3 minor releases** later)
+3. Do **not** use `openhands.sdk.utils.deprecation.deprecated` for FastAPI routes.
+   That decorator affects Python warnings/docstrings, not OpenAPI, and may be a
+   no-op before the declared deprecation version.
 
 Example:
 
@@ -50,9 +53,14 @@ 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 using [oasdiff](https://github.com/oasdiff/oasdiff).
+schema against the previous `openhands-agent-server` release selected from PyPI,
+but generates the baseline schema from the matching git tag under the current
+workspace dependency set before diffing with [oasdiff](https://github.com/oasdiff/oasdiff).
 
 It currently enforces:
+- FastAPI route handlers must not use `openhands.sdk.utils.deprecation.deprecated`.
+- Endpoints that document deprecation in their OpenAPI description must also set
+  `deprecated: true`.
 - No removal of operations (path + method) unless they were already marked
   `deprecated: true` in the previous release.
 - Breaking changes require a MINOR (or MAJOR) version bump.

tests/ci_scripts/test_check_agent_server_rest_api_breakage.py

@@ -24,6 +24,9 @@ def _load_prod_module():
 _prod = _load_prod_module()
 
 _find_deprecation_policy_errors = _prod._find_deprecation_policy_errors
+_find_sdk_deprecated_fastapi_routes_in_file = (
+    _prod._find_sdk_deprecated_fastapi_routes_in_file
+)
 _get_baseline_version = _prod._get_baseline_version
 _is_minor_or_major_bump = _prod._is_minor_or_major_bump
 _normalize_openapi_for_oasdiff = _prod._normalize_openapi_for_oasdiff
@@ -87,6 +90,68 @@ def test_find_deprecation_policy_errors_ignores_non_deprecated_operations():
     assert _find_deprecation_policy_errors(schema) == []
 
 
+def test_find_sdk_deprecated_fastapi_routes_in_file_flags_direct_import(tmp_path):
+    repo_root = tmp_path
+    source = repo_root / "openhands-agent-server" / "openhands" / "agent_server"
+    source.mkdir(parents=True)
+    file_path = source / "router.py"
+    file_path.write_text(
+        "from openhands.sdk.utils.deprecation import deprecated\n"
+        "\n"
+        '@router.get("/foo")\n'
+        '@deprecated(deprecated_in="1.0.0", removed_in="1.1.0")\n'
+        "async def foo():\n"
+        "    return {}\n"
+    )
+
+    errors = _find_sdk_deprecated_fastapi_routes_in_file(file_path, repo_root)
+
+    assert errors == [
+        "openhands-agent-server/openhands/agent_server/router.py:5 FastAPI route "
+        "`foo` uses openhands.sdk.utils.deprecation.deprecated; use the route "
+        "decorator's deprecated=True flag instead."
+    ]
+
+
+def test_find_sdk_deprecated_fastapi_routes_in_file_flags_alias_import(tmp_path):
+    repo_root = tmp_path
+    source = repo_root / "openhands-agent-server" / "openhands" / "agent_server"
+    source.mkdir(parents=True)
+    file_path = source / "router.py"
+    file_path.write_text(
+        "import openhands.sdk.utils.deprecation as dep\n"
+        "\n"
+        '@router.post("/foo")\n'
+        '@dep.deprecated(deprecated_in="1.0.0", removed_in="1.1.0")\n'
+        "async def foo():\n"
+        "    return {}\n"
+    )
+
+    errors = _find_sdk_deprecated_fastapi_routes_in_file(file_path, repo_root)
+
+    assert errors == [
+        "openhands-agent-server/openhands/agent_server/router.py:5 FastAPI route "
+        "`foo` uses openhands.sdk.utils.deprecation.deprecated; use the route "
+        "decorator's deprecated=True flag instead."
+    ]
+
+
+def test_find_sdk_deprecated_fastapi_routes_in_file_ignores_non_route_usage(tmp_path):
+    repo_root = tmp_path
+    source = repo_root / "openhands-agent-server" / "openhands" / "agent_server"
+    source.mkdir(parents=True)
+    file_path = source / "helpers.py"
+    file_path.write_text(
+        "from openhands.sdk.utils.deprecation import deprecated\n"
+        "\n"
+        '@deprecated(deprecated_in="1.0.0", removed_in="1.1.0")\n'
+        "def helper():\n"
+        "    return None\n"
+    )
+
+    assert _find_sdk_deprecated_fastapi_routes_in_file(file_path, repo_root) == []
+
+
 def test_get_baseline_version_warns_and_returns_none_when_pypi_fails(
     monkeypatch, capsys
 ):