|
14 | 14 | - Endpoints documented as deprecated in their OpenAPI description must also be |
15 | 15 | marked `deprecated: true` in the generated schema. |
16 | 16 |
|
17 | | -2) Deprecation-before-removal |
| 17 | +2) Deprecation runway before removal |
18 | 18 | - If a REST operation (path + HTTP method) is removed, it must have been marked |
19 | | - `deprecated: true` in the baseline release. |
| 19 | + `deprecated: true` in the baseline release and its OpenAPI description must |
| 20 | + declare a scheduled removal version that has been reached by the current |
| 21 | + package version. |
20 | 22 |
|
21 | | -3) MINOR version bump |
22 | | - - If a breaking REST change is detected, the current version must be at least a |
23 | | - MINOR bump compared to the baseline release. |
| 23 | +3) No in-place contract breakage |
| 24 | + - Breaking REST contract changes that are not removals of previously-deprecated |
| 25 | + operations fail the check. REST clients need 5 minor releases of runway, so |
| 26 | + incompatible replacements must ship additively or behind a versioned contract |
| 27 | + until the scheduled removal version. |
24 | 28 |
|
25 | 29 | If the baseline release schema can't be generated (e.g., missing tag / repo issues), |
26 | 30 | the script emits a warning and exits successfully to avoid flaky CI. |
|
30 | 34 |
|
31 | 35 | import ast |
32 | 36 | import json |
| 37 | +import re |
33 | 38 | import subprocess |
34 | 39 | import sys |
35 | 40 | import tempfile |
|
43 | 48 | REPO_ROOT = Path(__file__).resolve().parents[2] |
44 | 49 | AGENT_SERVER_PYPROJECT = REPO_ROOT / "openhands-agent-server" / "pyproject.toml" |
45 | 50 | PYPI_DISTRIBUTION = "openhands-agent-server" |
| 51 | +REST_ROUTE_DEPRECATION_RE = re.compile( |
| 52 | + r"Deprecated since v(?P<deprecated>[0-9A-Za-z.+-]+)\s+" |
| 53 | + r"and scheduled for removal in v(?P<removed>[0-9A-Za-z.+-]+)\.?", |
| 54 | + re.IGNORECASE, |
| 55 | +) |
46 | 56 | HTTP_METHODS = { |
47 | 57 | "get", |
48 | 58 | "put", |
@@ -292,6 +302,124 @@ def _find_deprecation_policy_errors(schema: dict) -> list[str]: |
292 | 302 | return errors |
293 | 303 |
|
294 | 304 |
|
| 305 | +def _parse_openapi_deprecation_description( |
| 306 | + description: str | None, |
| 307 | +) -> tuple[str, str] | None: |
| 308 | + if not description: |
| 309 | + return None |
| 310 | + |
| 311 | + match = REST_ROUTE_DEPRECATION_RE.search(" ".join(description.split())) |
| 312 | + if match is None: |
| 313 | + return None |
| 314 | + |
| 315 | + return match.group("deprecated").rstrip("."), match.group("removed").rstrip(".") |
| 316 | + |
| 317 | + |
| 318 | +def _version_ge(current: str, target: str) -> bool: |
| 319 | + try: |
| 320 | + return pkg_version.parse(current) >= pkg_version.parse(target) |
| 321 | + except pkg_version.InvalidVersion as exc: |
| 322 | + raise SystemExit( |
| 323 | + f"Invalid semantic version comparison: {current=} {target=}" |
| 324 | + ) from exc |
| 325 | + |
| 326 | + |
| 327 | +def _get_openapi_operation(schema: dict, path: str, method: str) -> dict | None: |
| 328 | + path_item = schema.get("paths", {}).get(path) |
| 329 | + if not isinstance(path_item, dict): |
| 330 | + return None |
| 331 | + |
| 332 | + operation = path_item.get(method.lower()) |
| 333 | + if not isinstance(operation, dict): |
| 334 | + return None |
| 335 | + |
| 336 | + return operation |
| 337 | + |
| 338 | + |
| 339 | +def _validate_removed_operations( |
| 340 | + removed_operations: list[dict], |
| 341 | + prev_schema: dict, |
| 342 | + current_version: str, |
| 343 | +) -> list[str]: |
| 344 | + errors: list[str] = [] |
| 345 | + |
| 346 | + for operation in removed_operations: |
| 347 | + path = str(operation.get("path", "")) |
| 348 | + method = str(operation.get("method", "")).lower() |
| 349 | + method_label = method.upper() or "<unknown method>" |
| 350 | + |
| 351 | + if not operation.get("deprecated", False): |
| 352 | + errors.append( |
| 353 | + f"Removed {method_label} {path} without prior deprecation " |
| 354 | + "(deprecated=true)." |
| 355 | + ) |
| 356 | + continue |
| 357 | + |
| 358 | + baseline_operation = _get_openapi_operation(prev_schema, path, method) |
| 359 | + if baseline_operation is None: |
| 360 | + errors.append( |
| 361 | + f"Removed {method_label} {path} was marked deprecated in the " |
| 362 | + "baseline release, but the previous OpenAPI schema could not be " |
| 363 | + "inspected for its scheduled removal version." |
| 364 | + ) |
| 365 | + continue |
| 366 | + |
| 367 | + deprecation_details = _parse_openapi_deprecation_description( |
| 368 | + baseline_operation.get("description") |
| 369 | + ) |
| 370 | + if deprecation_details is None: |
| 371 | + errors.append( |
| 372 | + f"Removed {method_label} {path} was marked deprecated in the " |
| 373 | + "baseline release, but its OpenAPI description does not declare " |
| 374 | + "a scheduled removal version. REST API removals require 5 minor " |
| 375 | + "releases of deprecation runway." |
| 376 | + ) |
| 377 | + continue |
| 378 | + |
| 379 | + _, removed_in = deprecation_details |
| 380 | + if not _version_ge(current_version, removed_in): |
| 381 | + errors.append( |
| 382 | + f"Removed {method_label} {path} before its scheduled removal " |
| 383 | + f"version v{removed_in} (current version: v{current_version}). " |
| 384 | + "REST API removals require 5 minor releases of deprecation " |
| 385 | + "runway." |
| 386 | + ) |
| 387 | + continue |
| 388 | + |
| 389 | + print( |
| 390 | + f"::notice title={PYPI_DISTRIBUTION} REST API::Removed previously-" |
| 391 | + f"deprecated {method_label} {path} after its scheduled removal " |
| 392 | + f"version v{removed_in}." |
| 393 | + ) |
| 394 | + |
| 395 | + return errors |
| 396 | + |
| 397 | + |
| 398 | +def _split_breaking_changes( |
| 399 | + breaking_changes: list[dict], |
| 400 | +) -> tuple[list[dict], list[dict]]: |
| 401 | + removed_operations: list[dict] = [] |
| 402 | + other_breaking_changes: list[dict] = [] |
| 403 | + |
| 404 | + for change in breaking_changes: |
| 405 | + change_id = str(change.get("id", "")) |
| 406 | + details = change.get("details", {}) |
| 407 | + |
| 408 | + if "removed" in change_id.lower() and "operation" in change_id.lower(): |
| 409 | + removed_operations.append( |
| 410 | + { |
| 411 | + "path": details.get("path", ""), |
| 412 | + "method": details.get("method", ""), |
| 413 | + "deprecated": details.get("deprecated", False), |
| 414 | + } |
| 415 | + ) |
| 416 | + continue |
| 417 | + |
| 418 | + other_breaking_changes.append(change) |
| 419 | + |
| 420 | + return removed_operations, other_breaking_changes |
| 421 | + |
| 422 | + |
295 | 423 | def _normalize_openapi_for_oasdiff(schema: dict) -> dict: |
296 | 424 | """Normalize OpenAPI 3.1 schema for oasdiff compatibility. |
297 | 425 |
|
@@ -373,14 +501,6 @@ def _run_oasdiff_breakage_check( |
373 | 501 | return breaking_changes, result.returncode |
374 | 502 |
|
375 | 503 |
|
376 | | -def _is_minor_or_major_bump(current: str, previous: str) -> bool: |
377 | | - cur = pkg_version.parse(current) |
378 | | - prev = pkg_version.parse(previous) |
379 | | - if cur <= prev: |
380 | | - return False |
381 | | - return (cur.major, cur.minor) != (prev.major, prev.minor) |
382 | | - |
383 | | - |
384 | 504 | def main() -> int: |
385 | 505 | current_version = _read_version_from_pyproject(AGENT_SERVER_PYPROJECT) |
386 | 506 | baseline_version = _get_baseline_version(PYPI_DISTRIBUTION, current_version) |
@@ -434,47 +554,38 @@ def main() -> int: |
434 | 554 | "in JSON format. There may be warnings only." |
435 | 555 | ) |
436 | 556 | else: |
437 | | - removed_operations = [] |
438 | | - |
439 | | - for change in breaking_changes: |
440 | | - change_id = change.get("id", "") |
441 | | - details = change.get("details", {}) |
442 | | - |
443 | | - if "removed" in change_id.lower() and "operation" in change_id.lower(): |
444 | | - removed_operations.append( |
445 | | - { |
446 | | - "path": details.get("path", ""), |
447 | | - "method": details.get("method", ""), |
448 | | - "deprecated": details.get("deprecated", False), |
449 | | - } |
450 | | - ) |
451 | | - |
452 | | - undeprecated_removals = [ |
453 | | - op for op in removed_operations if not op.get("deprecated", False) |
454 | | - ] |
455 | | - |
456 | | - for op in undeprecated_removals: |
457 | | - print( |
458 | | - f"::error title={PYPI_DISTRIBUTION} REST API::Removed " |
459 | | - f"{op['method'].upper()} {op['path']} without prior deprecation " |
460 | | - "(deprecated=true)." |
461 | | - ) |
| 557 | + removed_operations, other_breaking_changes = _split_breaking_changes( |
| 558 | + breaking_changes |
| 559 | + ) |
| 560 | + removal_errors = _validate_removed_operations( |
| 561 | + removed_operations, |
| 562 | + prev_schema, |
| 563 | + current_version, |
| 564 | + ) |
462 | 565 |
|
463 | | - if not _is_minor_or_major_bump(current_version, baseline_version): |
| 566 | + for error in removal_errors: |
| 567 | + print(f"::error title={PYPI_DISTRIBUTION} REST API::{error}") |
| 568 | + |
| 569 | + if other_breaking_changes: |
464 | 570 | print( |
465 | 571 | "::error " |
466 | | - f"title={PYPI_DISTRIBUTION} REST API::Breaking REST API change " |
467 | | - f"detected without MINOR version bump ({baseline_version} -> " |
468 | | - f"{current_version})." |
| 572 | + f"title={PYPI_DISTRIBUTION} REST API::Detected breaking REST API " |
| 573 | + "changes other than removing previously-deprecated operations. " |
| 574 | + "REST contract changes must preserve compatibility for 5 minor " |
| 575 | + "releases; keep the old contract available until its scheduled " |
| 576 | + "removal version." |
469 | 577 | ) |
470 | 578 |
|
471 | 579 | print("\nBreaking REST API changes detected compared to baseline release:") |
472 | 580 | for text in breaking_changes: |
473 | 581 | print(f"- {text.get('text', str(text))}") |
474 | 582 |
|
475 | | - if undeprecated_removals or not _is_minor_or_major_bump( |
476 | | - current_version, baseline_version |
477 | | - ): |
| 583 | + if not (removal_errors or other_breaking_changes): |
| 584 | + print( |
| 585 | + "Breaking changes are limited to previously-deprecated operations " |
| 586 | + "whose scheduled removal versions have been reached." |
| 587 | + ) |
| 588 | + else: |
478 | 589 | return 1 |
479 | 590 |
|
480 | 591 | return 1 if (static_policy_errors or deprecation_policy_errors) else 0 |
|
0 commit comments