From 0019f0378da037049f18cea554667b4a3878185d Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Mon, 16 Mar 2026 23:06:55 +0100 Subject: [PATCH 1/7] docs: document RFC 9457 dual response formats in OpenAPI schema Add ProblemDetail schema and dual content-type error responses (application/json envelope + application/problem+json bare) to the OpenAPI spec via a post-processing step in the export pipeline. Closes #494 --- scripts/export_openapi.py | 2 + src/synthorg/api/openapi.py | 402 ++++++++++++++++++++++++++ tests/unit/api/test_openapi.py | 496 +++++++++++++++++++++++++++++++++ 3 files changed, 900 insertions(+) create mode 100644 src/synthorg/api/openapi.py create mode 100644 tests/unit/api/test_openapi.py diff --git a/scripts/export_openapi.py b/scripts/export_openapi.py index 1a9ee9ca87..2ea7552c2d 100644 --- a/scripts/export_openapi.py +++ b/scripts/export_openapi.py @@ -91,9 +91,11 @@ def main() -> int: """Instantiate the app, extract the OpenAPI schema, and write files.""" try: from synthorg.api.app import create_app + from synthorg.api.openapi import inject_rfc9457_responses app = create_app() schema_dict = app.openapi_schema.to_schema() + schema_dict = inject_rfc9457_responses(schema_dict) schema_json = json.dumps(schema_dict, indent=2, ensure_ascii=False) + "\n" except Exception as exc: print("Failed to export OpenAPI schema:", file=sys.stderr) diff --git a/src/synthorg/api/openapi.py b/src/synthorg/api/openapi.py new file mode 100644 index 0000000000..f59810e820 --- /dev/null +++ b/src/synthorg/api/openapi.py @@ -0,0 +1,402 @@ +"""OpenAPI schema post-processor for RFC 9457 dual-format error responses. + +Litestar auto-generates the OpenAPI schema from controller return types, +but exception handlers (which perform content negotiation between +``application/json`` envelopes and ``application/problem+json`` bare +bodies) are invisible to the generator. + +This module provides :func:`inject_rfc9457_responses` which transforms +the Litestar-generated schema dict to: + +1. Add the ``ProblemDetail`` schema (RFC 9457 bare response body) +2. Define reusable error responses with dual content types +3. Inject error response references into every operation +4. Replace Litestar's default 400 schema with the actual envelope +5. Document content negotiation in ``info.description`` + +Called by ``scripts/export_openapi.py`` after schema generation. +""" + +import copy +from typing import Any, Final + +from synthorg.api.dto import ProblemDetail +from synthorg.api.errors import ( + CATEGORY_TITLES, + ErrorCategory, + ErrorCode, + category_type_uri, +) + +# ── Constants ───────────────────────────────────────────────── + +_PROBLEM_JSON: Final[str] = "application/problem+json" +_APP_JSON: Final[str] = "application/json" + +# Paths that skip authentication (no 401/403 injected). +_PUBLIC_PATH_SUFFIXES: Final[tuple[str, ...]] = ( + "/health", + "/auth/setup", + "/auth/login", +) + +# HTTP methods that accept a request body (get 400/409 injected). +_WRITE_METHODS: Final[frozenset[str]] = frozenset({"post", "put", "patch", "delete"}) + +# Envelope schema ref (Litestar-generated name for ApiResponse[None]). +_ENVELOPE_REF: Final[str] = "#/components/schemas/ApiResponse_NoneType_" + +# ProblemDetail schema ref (we add this). +_PROBLEM_DETAIL_REF: Final[str] = "#/components/schemas/ProblemDetail" + +_EXAMPLE_INSTANCE_ID: Final[str] = "a1b2c3d4-e5f6-7890-abcd-ef1234567890" + +# ── Error response definitions ──────────────────────────────── + +# (status_code, key, description, error_code, error_category, detail, retryable) +_ERROR_RESPONSES: Final[ + tuple[tuple[int, str, str, ErrorCode, ErrorCategory, str, bool], ...] +] = ( + ( + 400, + "BadRequest", + "Validation error — request body or parameters are invalid.", + ErrorCode.REQUEST_VALIDATION_ERROR, + ErrorCategory.VALIDATION, + "Validation error", + False, + ), + ( + 401, + "Unauthorized", + "Authentication required — missing or invalid credentials.", + ErrorCode.UNAUTHORIZED, + ErrorCategory.AUTH, + "Authentication required", + False, + ), + ( + 403, + "Forbidden", + "Insufficient permissions for this operation.", + ErrorCode.FORBIDDEN, + ErrorCategory.AUTH, + "Forbidden", + False, + ), + ( + 404, + "NotFound", + "Requested resource does not exist.", + ErrorCode.RECORD_NOT_FOUND, + ErrorCategory.NOT_FOUND, + "Resource not found", + False, + ), + ( + 409, + "Conflict", + "Resource conflict — duplicate or invalid state transition.", + ErrorCode.RESOURCE_CONFLICT, + ErrorCategory.CONFLICT, + "Resource conflict", + False, + ), + ( + 429, + "TooManyRequests", + "Rate limit exceeded — back off and retry.", + ErrorCode.RATE_LIMITED, + ErrorCategory.RATE_LIMIT, + "Rate limit exceeded", + True, + ), + ( + 500, + "InternalError", + "Internal server error.", + ErrorCode.INTERNAL_ERROR, + ErrorCategory.INTERNAL, + "Internal server error", + False, + ), + ( + 503, + "ServiceUnavailable", + "Required service is temporarily unavailable.", + ErrorCode.SERVICE_UNAVAILABLE, + ErrorCategory.INTERNAL, + "Service unavailable", + True, + ), +) + +_INFO_DESCRIPTION: Final[str] = """\ +SynthOrg REST API for managing synthetic organizations \u2014 autonomous \ +AI agents orchestrated as a virtual company. + +## Error Handling (RFC 9457) + +All error responses support content negotiation between two formats: + +- **`application/json`** (default): Standard `ApiResponse` envelope with \ +`error`, `error_detail`, and `success` fields +- **`application/problem+json`**: Bare RFC 9457 Problem Detail body \u2014 \ +send `Accept: application/problem+json` + +Every error includes machine-readable metadata: `error_code` \ +(4-digit category-grouped), `error_category`, `retryable`, and \ +`retry_after` (seconds). + +See the [Error Reference](https://synthorg.io/docs/errors) for the \ +full error taxonomy and retry guidance.\ +""" + + +# ── Helpers ─────────────────────────────────────────────────── + + +def _build_problem_detail_schema() -> dict[str, Any]: + """Generate the ``ProblemDetail`` JSON Schema from the Pydantic model. + + Rewrites internal ``$defs`` references to point at + ``#/components/schemas/`` so they resolve correctly when placed + inside the OpenAPI ``components.schemas`` section. + + Returns: + A two-tuple-style dict: the schema itself has ``$defs`` + stripped and ``$ref`` paths rewritten. + """ + raw = ProblemDetail.model_json_schema(mode="serialization") + + # Strip $defs — they'll be merged separately. + raw.pop("$defs", None) + + # Rewrite $ref from '#/$defs/X' to '#/components/schemas/X'. + result: dict[str, Any] = _rewrite_refs(raw) + return result + + +def _rewrite_refs(obj: Any) -> Any: + """Recursively rewrite ``$ref`` paths from Pydantic to OpenAPI.""" + if isinstance(obj, dict): + if "$ref" in obj: + ref: str = obj["$ref"] + if ref.startswith("#/$defs/"): + return {"$ref": f"#/components/schemas/{ref.removeprefix('#/$defs/')}"} + return {k: _rewrite_refs(v) for k, v in obj.items()} + if isinstance(obj, list): + return [_rewrite_refs(item) for item in obj] + return obj + + +def _envelope_example( + *, + detail: str, + error_code: ErrorCode, + error_category: ErrorCategory, + retryable: bool, +) -> dict[str, Any]: + """Build an ``ApiResponse`` envelope example for an error response.""" + title = CATEGORY_TITLES[error_category] + type_uri = category_type_uri(error_category) + return { + "data": None, + "error": detail, + "error_detail": { + "detail": detail, + "error_code": error_code.value, + "error_category": error_category.value, + "retryable": retryable, + "retry_after": None, + "instance": _EXAMPLE_INSTANCE_ID, + "title": title, + "type": type_uri, + }, + "success": False, + } + + +def _problem_detail_example( + *, + status: int, + detail: str, + error_code: ErrorCode, + error_category: ErrorCategory, + retryable: bool, +) -> dict[str, Any]: + """Build a bare RFC 9457 ``ProblemDetail`` example.""" + title = CATEGORY_TITLES[error_category] + type_uri = category_type_uri(error_category) + return { + "type": type_uri, + "title": title, + "status": status, + "detail": detail, + "instance": _EXAMPLE_INSTANCE_ID, + "error_code": error_code.value, + "error_category": error_category.value, + "retryable": retryable, + "retry_after": None, + } + + +def _build_reusable_response( # noqa: PLR0913 + *, + status: int, + description: str, + error_code: ErrorCode, + error_category: ErrorCategory, + detail: str, + retryable: bool, +) -> dict[str, Any]: + """Build a reusable response object with dual content types.""" + return { + "description": description, + "content": { + _APP_JSON: { + "schema": {"$ref": _ENVELOPE_REF}, + "example": _envelope_example( + detail=detail, + error_code=error_code, + error_category=error_category, + retryable=retryable, + ), + }, + _PROBLEM_JSON: { + "schema": {"$ref": _PROBLEM_DETAIL_REF}, + "example": _problem_detail_example( + status=status, + detail=detail, + error_code=error_code, + error_category=error_category, + retryable=retryable, + ), + }, + }, + } + + +def _is_public_path(path: str) -> bool: + """Check whether a path is unauthenticated (no 401/403).""" + return any(path.endswith(suffix) for suffix in _PUBLIC_PATH_SUFFIXES) + + +def _has_path_params(path: str) -> bool: + """Check whether a path contains ``{param}`` segments.""" + return "{" in path + + +def _response_ref(key: str) -> dict[str, str]: + """Build a ``$ref`` to a reusable response.""" + return {"$ref": f"#/components/responses/{key}"} + + +# ── Response-to-operation mapping ───────────────────────────── + +# Maps response keys to the condition under which they are injected. +# Condition signature: (path: str, method: str, operation: dict) -> bool + + +def _should_inject( + key: str, + path: str, + method: str, + operation: dict[str, Any], +) -> bool: + """Decide whether to inject a response reference into an operation. + + Returns ``True`` when the given error response *key* is applicable + to the *path*/*method* combination. + """ + is_public = _is_public_path(path) + is_write = method in _WRITE_METHODS + has_params = _has_path_params(path) + + checks: dict[str, bool] = { + "InternalError": True, + "ServiceUnavailable": not is_public, + "Unauthorized": not is_public, + "Forbidden": not is_public and is_write, + # Inject on write methods or replace Litestar's incorrect default. + "BadRequest": is_write or "400" in operation.get("responses", {}), + "NotFound": has_params, + "Conflict": method in {"post", "put", "patch"}, + "TooManyRequests": not is_public, + } + return checks.get(key, False) + + +# ── Main function ───────────────────────────────────────────── + + +def inject_rfc9457_responses(schema: dict[str, Any]) -> dict[str, Any]: + """Inject RFC 9457 dual-format error responses into an OpenAPI schema. + + Takes the raw schema dict produced by Litestar's + ``app.openapi_schema.to_schema()`` and returns a **new** dict with: + + - ``ProblemDetail`` added to ``components.schemas`` + - Reusable error responses (dual content types) in + ``components.responses`` + - Error response refs injected into every operation + - ``info.description`` updated with RFC 9457 documentation + + Args: + schema: OpenAPI schema dict (not modified). + + Returns: + Enhanced copy of the schema. + """ + result = copy.deepcopy(schema) + + components = result.setdefault("components", {}) + schemas = components.setdefault("schemas", {}) + responses = components.setdefault("responses", {}) + + # 1. Add ProblemDetail schema. + if "ProblemDetail" not in schemas: + schemas["ProblemDetail"] = _build_problem_detail_schema() + + # 2. Build reusable responses with dual content types. + response_keys: list[str] = [] + for ( + status, + key, + description, + error_code, + error_category, + detail, + retryable, + ) in _ERROR_RESPONSES: + responses[key] = _build_reusable_response( + status=status, + description=description, + error_code=error_code, + error_category=error_category, + detail=detail, + retryable=retryable, + ) + response_keys.append(key) + + # 3. Inject into operations. + status_for_key = {key: str(status) for status, key, *_ in _ERROR_RESPONSES} + + for path, path_item in result.get("paths", {}).items(): + for method, operation in path_item.items(): + if not isinstance(operation, dict) or "responses" not in operation: + continue + op_responses = operation["responses"] + + for key in response_keys: + status_code = status_for_key[key] + if not _should_inject(key, path, method, operation): + continue + # Always replace 400 (Litestar's default is incorrect). + if status_code == "400" or status_code not in op_responses: + op_responses[status_code] = _response_ref(key) + + # 4. Update info.description. + result.setdefault("info", {})["description"] = _INFO_DESCRIPTION + + return result diff --git a/tests/unit/api/test_openapi.py b/tests/unit/api/test_openapi.py new file mode 100644 index 0000000000..e10318309d --- /dev/null +++ b/tests/unit/api/test_openapi.py @@ -0,0 +1,496 @@ +"""Tests for RFC 9457 OpenAPI schema post-processing. + +Verifies that :func:`inject_rfc9457_responses` correctly adds +``ProblemDetail`` schema, reusable error responses with dual +content types, and injects error references into operations. +""" + +import copy +from typing import Any + +import pytest + +from synthorg.api.openapi import inject_rfc9457_responses + +# ── Fixtures ────────────────────────────────────────────────── + + +def _minimal_operation( + *, + status: int = 200, + has_body: bool = False, +) -> dict[str, Any]: + """Build a minimal OpenAPI operation dict.""" + op: dict[str, Any] = { + "responses": { + str(status): { + "description": "OK", + "content": {"application/json": {"schema": {"type": "object"}}}, + }, + }, + } + if has_body: + op["requestBody"] = { + "content": {"application/json": {"schema": {"type": "object"}}}, + } + return op + + +def _minimal_schema( + *, + paths: dict[str, dict[str, Any]] | None = None, + extra_schemas: dict[str, Any] | None = None, +) -> dict[str, Any]: + """Build a minimal OpenAPI schema dict for testing.""" + schemas: dict[str, Any] = { + "ErrorCode": {"type": "integer", "enum": [1000, 3001]}, + "ErrorCategory": {"type": "string", "enum": ["auth", "not_found"]}, + "ErrorDetail": {"type": "object", "properties": {}}, + "ApiResponse_NoneType_": {"type": "object", "properties": {}}, + } + if extra_schemas: + schemas.update(extra_schemas) + return { + "openapi": "3.1.0", + "info": {"title": "Test API", "version": "0.1.0"}, + "paths": paths or {}, + "components": {"schemas": schemas}, + } + + +@pytest.fixture +def base_schema() -> dict[str, Any]: + """Schema with representative paths for injection testing.""" + return _minimal_schema( + paths={ + "/api/v1/health": { + "get": _minimal_operation(), + }, + "/api/v1/auth/login": { + "post": _minimal_operation(status=200, has_body=True), + }, + "/api/v1/auth/setup": { + "post": _minimal_operation(status=201, has_body=True), + }, + "/api/v1/tasks": { + "get": _minimal_operation(), + "post": _minimal_operation(status=201, has_body=True), + }, + "/api/v1/tasks/{task_id}": { + "get": _minimal_operation(), + "delete": _minimal_operation(), + "patch": _minimal_operation(has_body=True), + }, + "/api/v1/agents": { + "get": _minimal_operation(), + }, + "/api/v1/agents/{agent_name}": { + "get": _minimal_operation(), + }, + }, + ) + + +# ── ProblemDetail schema ───────────────────────────────────── + + +@pytest.mark.unit +class TestProblemDetailSchema: + """ProblemDetail schema is correctly added to components.""" + + def test_added_to_components(self, base_schema: dict[str, Any]) -> None: + result = inject_rfc9457_responses(base_schema) + schemas = result["components"]["schemas"] + assert "ProblemDetail" in schemas + + def test_has_required_fields(self, base_schema: dict[str, Any]) -> None: + result = inject_rfc9457_responses(base_schema) + pd = result["components"]["schemas"]["ProblemDetail"] + required = set(pd.get("required", [])) + expected = { + "type", + "title", + "status", + "detail", + "instance", + "error_code", + "error_category", + } + assert expected == required + + def test_has_all_properties(self, base_schema: dict[str, Any]) -> None: + result = inject_rfc9457_responses(base_schema) + pd = result["components"]["schemas"]["ProblemDetail"] + props = set(pd.get("properties", {}).keys()) + expected = { + "type", + "title", + "status", + "detail", + "instance", + "error_code", + "error_category", + "retryable", + "retry_after", + } + assert expected == props + + def test_reuses_existing_error_code_ref(self, base_schema: dict[str, Any]) -> None: + result = inject_rfc9457_responses(base_schema) + pd = result["components"]["schemas"]["ProblemDetail"] + error_code_prop = pd["properties"]["error_code"] + assert error_code_prop == {"$ref": "#/components/schemas/ErrorCode"} + + def test_reuses_existing_error_category_ref( + self, base_schema: dict[str, Any] + ) -> None: + result = inject_rfc9457_responses(base_schema) + pd = result["components"]["schemas"]["ProblemDetail"] + error_cat_prop = pd["properties"]["error_category"] + assert error_cat_prop == {"$ref": "#/components/schemas/ErrorCategory"} + + def test_not_overwritten_if_exists(self) -> None: + """Pre-existing ProblemDetail schema is preserved.""" + custom_pd = {"type": "object", "custom": True} + schema = _minimal_schema(extra_schemas={"ProblemDetail": custom_pd}) + result = inject_rfc9457_responses(schema) + assert result["components"]["schemas"]["ProblemDetail"]["custom"] is True + + +# ── Reusable responses ──────────────────────────────────────── + +_EXPECTED_RESPONSE_KEYS = frozenset( + { + "BadRequest", + "Unauthorized", + "Forbidden", + "NotFound", + "Conflict", + "TooManyRequests", + "InternalError", + "ServiceUnavailable", + } +) + + +@pytest.mark.unit +class TestReusableResponses: + """Reusable responses are defined with dual content types.""" + + def test_all_responses_defined(self, base_schema: dict[str, Any]) -> None: + result = inject_rfc9457_responses(base_schema) + responses = result["components"]["responses"] + assert set(responses.keys()) == _EXPECTED_RESPONSE_KEYS + + @pytest.mark.parametrize("key", sorted(_EXPECTED_RESPONSE_KEYS)) + def test_has_dual_content_types( + self, base_schema: dict[str, Any], key: str + ) -> None: + result = inject_rfc9457_responses(base_schema) + content = result["components"]["responses"][key]["content"] + assert "application/json" in content + assert "application/problem+json" in content + + @pytest.mark.parametrize("key", sorted(_EXPECTED_RESPONSE_KEYS)) + def test_json_content_refs_envelope( + self, base_schema: dict[str, Any], key: str + ) -> None: + result = inject_rfc9457_responses(base_schema) + json_content = result["components"]["responses"][key]["content"][ + "application/json" + ] + assert json_content["schema"] == { + "$ref": "#/components/schemas/ApiResponse_NoneType_" + } + + @pytest.mark.parametrize("key", sorted(_EXPECTED_RESPONSE_KEYS)) + def test_problem_json_refs_problem_detail( + self, base_schema: dict[str, Any], key: str + ) -> None: + result = inject_rfc9457_responses(base_schema) + pj_content = result["components"]["responses"][key]["content"][ + "application/problem+json" + ] + assert pj_content["schema"] == {"$ref": "#/components/schemas/ProblemDetail"} + + @pytest.mark.parametrize("key", sorted(_EXPECTED_RESPONSE_KEYS)) + def test_examples_present(self, base_schema: dict[str, Any], key: str) -> None: + result = inject_rfc9457_responses(base_schema) + content = result["components"]["responses"][key]["content"] + # Both content types must have an example. + assert "example" in content["application/json"] + assert "example" in content["application/problem+json"] + + @pytest.mark.parametrize("key", sorted(_EXPECTED_RESPONSE_KEYS)) + def test_envelope_example_structure( + self, base_schema: dict[str, Any], key: str + ) -> None: + result = inject_rfc9457_responses(base_schema) + example = result["components"]["responses"][key]["content"]["application/json"][ + "example" + ] + assert example["data"] is None + assert example["success"] is False + assert isinstance(example["error"], str) + assert isinstance(example["error_detail"], dict) + detail = example["error_detail"] + assert "error_code" in detail + assert "error_category" in detail + assert "instance" in detail + + @pytest.mark.parametrize("key", sorted(_EXPECTED_RESPONSE_KEYS)) + def test_problem_detail_example_structure( + self, base_schema: dict[str, Any], key: str + ) -> None: + result = inject_rfc9457_responses(base_schema) + example = result["components"]["responses"][key]["content"][ + "application/problem+json" + ]["example"] + assert "type" in example + assert "title" in example + assert "status" in example + assert isinstance(example["status"], int) + assert "detail" in example + assert "instance" in example + assert "error_code" in example + assert "error_category" in example + + +# ── Operation injection ─────────────────────────────────────── + + +@pytest.mark.unit +class TestOperationInjection: + """Error responses are injected into the correct operations.""" + + def test_all_operations_have_500(self, base_schema: dict[str, Any]) -> None: + result = inject_rfc9457_responses(base_schema) + for path, path_item in result["paths"].items(): + for method, operation in path_item.items(): + if not isinstance(operation, dict): + continue + responses = operation.get("responses", {}) + assert "500" in responses, f"{method.upper()} {path} missing 500" + assert responses["500"] == { + "$ref": "#/components/responses/InternalError" + } + + def test_authenticated_endpoints_have_401( + self, base_schema: dict[str, Any] + ) -> None: + result = inject_rfc9457_responses(base_schema) + # Non-public paths should have 401. + for path in ("/api/v1/tasks", "/api/v1/agents"): + for method in result["paths"][path]: + responses = result["paths"][path][method]["responses"] + assert "401" in responses, f"{method.upper()} {path} missing 401" + + def test_public_endpoints_skip_401(self, base_schema: dict[str, Any]) -> None: + result = inject_rfc9457_responses(base_schema) + for path in ("/api/v1/health", "/api/v1/auth/login", "/api/v1/auth/setup"): + for method in result["paths"][path]: + op = result["paths"][path][method] + if not isinstance(op, dict): + continue + responses = op.get("responses", {}) + assert "401" not in responses, f"{path} should not have 401" + assert "403" not in responses, f"{path} should not have 403" + + def test_path_param_endpoints_have_404(self, base_schema: dict[str, Any]) -> None: + result = inject_rfc9457_responses(base_schema) + for path in ("/api/v1/tasks/{task_id}", "/api/v1/agents/{agent_name}"): + for method in result["paths"][path]: + responses = result["paths"][path][method]["responses"] + assert "404" in responses, f"{method.upper()} {path} missing 404" + + def test_non_param_endpoints_skip_404(self, base_schema: dict[str, Any]) -> None: + result = inject_rfc9457_responses(base_schema) + for path in ("/api/v1/tasks", "/api/v1/agents"): + for method in result["paths"][path]: + responses = result["paths"][path][method]["responses"] + assert "404" not in responses, ( + f"{method.upper()} {path} should not have 404" + ) + + def test_write_endpoints_have_409(self, base_schema: dict[str, Any]) -> None: + result = inject_rfc9457_responses(base_schema) + # POST /tasks should have 409 + post_responses = result["paths"]["/api/v1/tasks"]["post"]["responses"] + assert "409" in post_responses + # PATCH should also have 409 (update can conflict). + patch_responses = result["paths"]["/api/v1/tasks/{task_id}"]["patch"][ + "responses" + ] + assert "409" in patch_responses + + def test_get_endpoints_skip_409(self, base_schema: dict[str, Any]) -> None: + result = inject_rfc9457_responses(base_schema) + get_responses = result["paths"]["/api/v1/tasks"]["get"]["responses"] + assert "409" not in get_responses + + def test_write_endpoints_have_403(self, base_schema: dict[str, Any]) -> None: + result = inject_rfc9457_responses(base_schema) + post_responses = result["paths"]["/api/v1/tasks"]["post"]["responses"] + assert "403" in post_responses + + def test_get_endpoints_skip_403(self, base_schema: dict[str, Any]) -> None: + result = inject_rfc9457_responses(base_schema) + get_responses = result["paths"]["/api/v1/tasks"]["get"]["responses"] + assert "403" not in get_responses + + def test_existing_responses_not_overwritten(self) -> None: + """Pre-existing non-400 error responses are preserved.""" + custom_404 = {"description": "Custom not found", "custom": True} + schema = _minimal_schema( + paths={ + "/api/v1/items/{id}": { + "get": { + "responses": { + "200": {"description": "OK"}, + "404": custom_404, + }, + }, + }, + }, + ) + result = inject_rfc9457_responses(schema) + resp_404 = result["paths"]["/api/v1/items/{id}"]["get"]["responses"]["404"] + # Should keep the custom response, not overwrite with ref. + assert resp_404.get("custom") is True + + def test_default_400_replaced(self) -> None: + """Litestar's default 400 (ValidationException) is replaced.""" + litestar_400 = { + "description": "Bad request", + "content": { + "application/json": { + "schema": { + "properties": { + "status_code": {"type": "integer"}, + "detail": {"type": "string"}, + "extra": {"type": ["null", "object"]}, + }, + "description": "Validation Exception", + }, + }, + }, + } + schema = _minimal_schema( + paths={ + "/api/v1/tasks": { + "get": { + "responses": { + "200": {"description": "OK"}, + "400": litestar_400, + }, + }, + }, + }, + ) + result = inject_rfc9457_responses(schema) + resp_400 = result["paths"]["/api/v1/tasks"]["get"]["responses"]["400"] + # Should be replaced with our ref. + assert resp_400 == {"$ref": "#/components/responses/BadRequest"} + + def test_non_public_have_429(self, base_schema: dict[str, Any]) -> None: + result = inject_rfc9457_responses(base_schema) + responses = result["paths"]["/api/v1/tasks"]["get"]["responses"] + assert "429" in responses + + def test_public_skip_429(self, base_schema: dict[str, Any]) -> None: + result = inject_rfc9457_responses(base_schema) + responses = result["paths"]["/api/v1/health"]["get"]["responses"] + assert "429" not in responses + + +# ── Info description ────────────────────────────────────────── + + +@pytest.mark.unit +class TestInfoDescription: + """info.description is updated with RFC 9457 documentation.""" + + def test_mentions_rfc_9457(self, base_schema: dict[str, Any]) -> None: + result = inject_rfc9457_responses(base_schema) + desc = result["info"]["description"] + assert "RFC 9457" in desc + + def test_mentions_content_negotiation(self, base_schema: dict[str, Any]) -> None: + result = inject_rfc9457_responses(base_schema) + desc = result["info"]["description"] + assert "application/problem+json" in desc + assert "application/json" in desc + + def test_mentions_error_reference(self, base_schema: dict[str, Any]) -> None: + result = inject_rfc9457_responses(base_schema) + desc = result["info"]["description"] + assert "synthorg.io/docs/errors" in desc + + +# ── Idempotency and immutability ────────────────────────────── + + +@pytest.mark.unit +class TestIdempotencyAndImmutability: + """Function is idempotent and does not mutate the input.""" + + def test_idempotent(self, base_schema: dict[str, Any]) -> None: + first = inject_rfc9457_responses(base_schema) + second = inject_rfc9457_responses(first) + assert first == second + + def test_does_not_mutate_input(self, base_schema: dict[str, Any]) -> None: + original = copy.deepcopy(base_schema) + inject_rfc9457_responses(base_schema) + assert base_schema == original + + def test_empty_paths(self) -> None: + """Handles schema with no paths gracefully.""" + schema = _minimal_schema(paths={}) + result = inject_rfc9457_responses(schema) + assert "ProblemDetail" in result["components"]["schemas"] + assert len(result["components"]["responses"]) == 8 + + def test_missing_components(self) -> None: + """Handles schema with missing components section.""" + schema = {"openapi": "3.1.0", "info": {"title": "X", "version": "1"}} + result = inject_rfc9457_responses(schema) + assert "ProblemDetail" in result["components"]["schemas"] + + +# ── Integration ─────────────────────────────────────────────── + + +@pytest.mark.unit +def test_full_app_schema_enhancement() -> None: + """Integration: enhance the real Litestar-generated schema.""" + from synthorg.api.app import create_app + + app = create_app() + schema = app.openapi_schema.to_schema() + result = inject_rfc9457_responses(schema) + + # ProblemDetail schema present. + assert "ProblemDetail" in result["components"]["schemas"] + + # All 8 reusable responses defined. + responses = result["components"]["responses"] + assert set(responses.keys()) == _EXPECTED_RESPONSE_KEYS + + # Every reusable response has dual content types. + for key, resp in responses.items(): + content = resp["content"] + assert "application/json" in content, f"{key} missing application/json" + assert "application/problem+json" in content, f"{key} missing problem+json" + + # At least one operation has error response refs. + tasks_get = result["paths"]["/api/v1/tasks"]["get"]["responses"] + assert "500" in tasks_get + assert tasks_get["500"] == {"$ref": "#/components/responses/InternalError"} + + # Public endpoints don't have 401. + health = result["paths"]["/api/v1/health"]["get"]["responses"] + assert "401" not in health + + # Info description updated. + assert "RFC 9457" in result["info"]["description"] From 48f2661b08bffc2478c19c03b65bedbf574cea38 Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Mon, 16 Mar 2026 23:21:56 +0100 Subject: [PATCH 2/7] refactor: address review findings from 7 agents - Fix misleading docstring return description in _build_problem_detail_schema - Extract _inject_operation_responses helper (function length + nesting) - Add comment explaining DELETE exclusion from Conflict (409) - Change test_full_app_schema_enhancement marker to @pytest.mark.integration - Add tests: DELETE skips 409, public paths skip 503, retryable examples - Update CLAUDE.md package structure and quick commands for openapi.py Pre-reviewed by 7 agents, 9 findings addressed --- CLAUDE.md | 4 +-- src/synthorg/api/openapi.py | 48 ++++++++++++++++++++++------------ tests/unit/api/test_openapi.py | 39 +++++++++++++++++++++++++-- 3 files changed, 70 insertions(+), 21 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 7e260997a6..67bbdff2bf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -39,7 +39,7 @@ uv run python -m pytest tests/ -m e2e -n auto # e2e tests only uv run python -m pytest tests/ -n auto --cov=synthorg --cov-fail-under=80 # full suite + coverage HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties # property tests (dev profile, 1000 examples) uv run pre-commit run --all-files # all pre-commit hooks -uv run python scripts/export_openapi.py # export OpenAPI schema (needed before docs build) +uv run python scripts/export_openapi.py # export OpenAPI schema with RFC 9457 error responses (needed before docs build) uv run zensical build # build docs (output: _site/docs/) — no --strict until zensical/backlog#72 uv run zensical serve # local docs preview (http://127.0.0.1:8000) ``` @@ -114,7 +114,7 @@ curl http://localhost:3000/api/v1/health # backend (via web proxy) ```text src/synthorg/ - api/ # Litestar REST + WebSocket API (controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, RFC 9457 structured errors (ErrorCategory, ErrorCode, ErrorDetail, ProblemDetail, CATEGORY_TITLES, category_title, category_type_uri, content negotiation)) + api/ # Litestar REST + WebSocket API (controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, RFC 9457 structured errors (ErrorCategory, ErrorCode, ErrorDetail, ProblemDetail, CATEGORY_TITLES, category_title, category_type_uri, content negotiation), OpenAPI schema post-processor (inject_rfc9457_responses — dual-format error responses for export)) auth/ # Authentication subpackage (controller, service, middleware, JWT + API key + WS ticket store, models, config) budget/ # Cost tracking, budget enforcement (pre-flight/in-flight checks, auto-downgrade), billing periods, cost tiers, quota/subscription tracking, CFO cost optimization (anomaly detection, efficiency analysis, downgrade recommendations, approval decisions), spending reports, budget errors (BudgetExhaustedError, DailyLimitExceededError, QuotaExhaustedError) cli/ # Python CLI module (superseded by top-level cli/ Go binary) diff --git a/src/synthorg/api/openapi.py b/src/synthorg/api/openapi.py index f59810e820..371527b2f1 100644 --- a/src/synthorg/api/openapi.py +++ b/src/synthorg/api/openapi.py @@ -164,8 +164,9 @@ def _build_problem_detail_schema() -> dict[str, Any]: inside the OpenAPI ``components.schemas`` section. Returns: - A two-tuple-style dict: the schema itself has ``$defs`` - stripped and ``$ref`` paths rewritten. + JSON Schema dict for ``ProblemDetail`` with ``$defs`` + stripped and ``$ref`` paths rewritten to resolve under + ``#/components/schemas/``. """ raw = ProblemDetail.model_json_schema(mode="serialization") @@ -321,12 +322,38 @@ def _should_inject( # Inject on write methods or replace Litestar's incorrect default. "BadRequest": is_write or "400" in operation.get("responses", {}), "NotFound": has_params, + # DELETE is excluded — it's idempotent and conflicts are + # a create/update concern in this API's model. "Conflict": method in {"post", "put", "patch"}, "TooManyRequests": not is_public, } return checks.get(key, False) +def _inject_operation_responses( + paths: dict[str, Any], + response_keys: list[str], + status_for_key: dict[str, str], +) -> None: + """Inject error response refs into each operation in *paths*. + + Mutates *paths* in place — the caller is responsible for + passing a deep-copied schema. + """ + for path, path_item in paths.items(): + for method, operation in path_item.items(): + if not isinstance(operation, dict) or "responses" not in operation: + continue + op_responses = operation["responses"] + for key in response_keys: + status_code = status_for_key[key] + if not _should_inject(key, path, method, operation): + continue + # Always replace 400 (Litestar's default is incorrect). + if status_code == "400" or status_code not in op_responses: + op_responses[status_code] = _response_ref(key) + + # ── Main function ───────────────────────────────────────────── @@ -379,22 +406,9 @@ def inject_rfc9457_responses(schema: dict[str, Any]) -> dict[str, Any]: ) response_keys.append(key) - # 3. Inject into operations. + # 3. Inject error response refs into individual operations. status_for_key = {key: str(status) for status, key, *_ in _ERROR_RESPONSES} - - for path, path_item in result.get("paths", {}).items(): - for method, operation in path_item.items(): - if not isinstance(operation, dict) or "responses" not in operation: - continue - op_responses = operation["responses"] - - for key in response_keys: - status_code = status_for_key[key] - if not _should_inject(key, path, method, operation): - continue - # Always replace 400 (Litestar's default is incorrect). - if status_code == "400" or status_code not in op_responses: - op_responses[status_code] = _response_ref(key) + _inject_operation_responses(result.get("paths", {}), response_keys, status_for_key) # 4. Update info.description. result.setdefault("info", {})["description"] = _INFO_DESCRIPTION diff --git a/tests/unit/api/test_openapi.py b/tests/unit/api/test_openapi.py index e10318309d..e506d11ab1 100644 --- a/tests/unit/api/test_openapi.py +++ b/tests/unit/api/test_openapi.py @@ -402,6 +402,41 @@ def test_public_skip_429(self, base_schema: dict[str, Any]) -> None: responses = result["paths"]["/api/v1/health"]["get"]["responses"] assert "429" not in responses + def test_public_skip_503(self, base_schema: dict[str, Any]) -> None: + result = inject_rfc9457_responses(base_schema) + responses = result["paths"]["/api/v1/health"]["get"]["responses"] + assert "503" not in responses + + def test_delete_skip_409(self, base_schema: dict[str, Any]) -> None: + """DELETE is idempotent — conflicts are a create/update concern.""" + result = inject_rfc9457_responses(base_schema) + responses = result["paths"]["/api/v1/tasks/{task_id}"]["delete"]["responses"] + assert "409" not in responses + + @pytest.mark.parametrize("key", ["TooManyRequests", "ServiceUnavailable"]) + def test_retryable_example_has_retryable_true( + self, base_schema: dict[str, Any], key: str + ) -> None: + result = inject_rfc9457_responses(base_schema) + content = result["components"]["responses"][key]["content"] + envelope_ex = content["application/json"]["example"] + assert envelope_ex["error_detail"]["retryable"] is True + problem_ex = content["application/problem+json"]["example"] + assert problem_ex["retryable"] is True + + @pytest.mark.parametrize( + "key", ["BadRequest", "Unauthorized", "Forbidden", "NotFound", "Conflict"] + ) + def test_non_retryable_example_has_retryable_false( + self, base_schema: dict[str, Any], key: str + ) -> None: + result = inject_rfc9457_responses(base_schema) + content = result["components"]["responses"][key]["content"] + envelope_ex = content["application/json"]["example"] + assert envelope_ex["error_detail"]["retryable"] is False + problem_ex = content["application/problem+json"]["example"] + assert problem_ex["retryable"] is False + # ── Info description ────────────────────────────────────────── @@ -461,9 +496,9 @@ def test_missing_components(self) -> None: # ── Integration ─────────────────────────────────────────────── -@pytest.mark.unit +@pytest.mark.integration def test_full_app_schema_enhancement() -> None: - """Integration: enhance the real Litestar-generated schema.""" + """Enhance the real Litestar-generated schema end-to-end.""" from synthorg.api.app import create_app app = create_app() From 3a7b4c29eabb7282f170771f3bf7e270844d0d00 Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Mon, 16 Mar 2026 23:58:00 +0100 Subject: [PATCH 3/7] refactor: address 16 review findings from CodeRabbit, Gemini, and 8 local agents Source changes (openapi.py): - Add logger declaration (CodeRabbit + code-reviewer + conventions-enforcer) - Replace raw tuples with NamedTuple for _ERROR_RESPONSES (Gemini + code-reviewer) - Extract 4 helpers from inject_rfc9457_responses to fix 50-line limit (code-reviewer) - Preserve existing info.description and append RFC section idempotently (CodeRabbit) - Simplify Conflict check: is_write and method != "delete" (Gemini) - Expand 400-replacement comment documenting intentional behavior (CodeRabbit) - Update _rewrite_refs and _build_problem_detail_schema docstrings (code-reviewer) Test changes: - Add 13 new tests (98 total): status-matches-HTTP-code, PUT method injection, BadRequest negative case on GET, unknown key fallback, $defs absence, path-level parameters guard, description preservation - Move integration test from tests/unit/ to tests/integration/api/ (code-reviewer) - Use issubset for integration response key check (CodeRabbit) --- src/synthorg/api/openapi.py | 309 ++++++++++-------- tests/integration/api/__init__.py | 0 .../api/test_openapi_integration.py | 63 ++++ tests/unit/api/test_openapi.py | 162 ++++++--- 4 files changed, 352 insertions(+), 182 deletions(-) create mode 100644 tests/integration/api/__init__.py create mode 100644 tests/integration/api/test_openapi_integration.py diff --git a/src/synthorg/api/openapi.py b/src/synthorg/api/openapi.py index 371527b2f1..2e0133caa3 100644 --- a/src/synthorg/api/openapi.py +++ b/src/synthorg/api/openapi.py @@ -12,13 +12,20 @@ 2. Define reusable error responses with dual content types 3. Inject error response references into every operation 4. Replace Litestar's default 400 schema with the actual envelope -5. Document content negotiation in ``info.description`` +5. Append content negotiation docs to ``info.description`` Called by ``scripts/export_openapi.py`` after schema generation. + +.. note:: + + The ``ProblemDetail`` schema rewrites ``$ref`` paths from Pydantic's + internal ``#/$defs/`` to ``#/components/schemas/``. This assumes + the referenced schemas (``ErrorCode``, ``ErrorCategory``) already + exist in the Litestar-generated ``components.schemas``. """ import copy -from typing import Any, Final +from typing import Any, Final, NamedTuple from synthorg.api.dto import ProblemDetail from synthorg.api.errors import ( @@ -27,6 +34,9 @@ ErrorCode, category_type_uri, ) +from synthorg.observability import get_logger + +logger = get_logger(__name__) # ── Constants ───────────────────────────────────────────────── @@ -41,7 +51,9 @@ ) # HTTP methods that accept a request body (get 400/409 injected). -_WRITE_METHODS: Final[frozenset[str]] = frozenset({"post", "put", "patch", "delete"}) +_WRITE_METHODS: Final[frozenset[str]] = frozenset( + {"post", "put", "patch", "delete"}, +) # Envelope schema ref (Litestar-generated name for ApiResponse[None]). _ENVELOPE_REF: Final[str] = "#/components/schemas/ApiResponse_NoneType_" @@ -53,88 +65,95 @@ # ── Error response definitions ──────────────────────────────── -# (status_code, key, description, error_code, error_category, detail, retryable) -_ERROR_RESPONSES: Final[ - tuple[tuple[int, str, str, ErrorCode, ErrorCategory, str, bool], ...] -] = ( - ( - 400, - "BadRequest", - "Validation error — request body or parameters are invalid.", - ErrorCode.REQUEST_VALIDATION_ERROR, - ErrorCategory.VALIDATION, - "Validation error", - False, + +class _ErrorResponseSpec(NamedTuple): + """Specification for a reusable error response definition.""" + + status: int + key: str + description: str + error_code: ErrorCode + error_category: ErrorCategory + detail: str + retryable: bool + + +_ERROR_RESPONSES: Final[tuple[_ErrorResponseSpec, ...]] = ( + _ErrorResponseSpec( + status=400, + key="BadRequest", + description=("Validation error \u2014 request body or parameters are invalid."), + error_code=ErrorCode.REQUEST_VALIDATION_ERROR, + error_category=ErrorCategory.VALIDATION, + detail="Validation error", + retryable=False, ), - ( - 401, - "Unauthorized", - "Authentication required — missing or invalid credentials.", - ErrorCode.UNAUTHORIZED, - ErrorCategory.AUTH, - "Authentication required", - False, + _ErrorResponseSpec( + status=401, + key="Unauthorized", + description=("Authentication required \u2014 missing or invalid credentials."), + error_code=ErrorCode.UNAUTHORIZED, + error_category=ErrorCategory.AUTH, + detail="Authentication required", + retryable=False, ), - ( - 403, - "Forbidden", - "Insufficient permissions for this operation.", - ErrorCode.FORBIDDEN, - ErrorCategory.AUTH, - "Forbidden", - False, + _ErrorResponseSpec( + status=403, + key="Forbidden", + description="Insufficient permissions for this operation.", + error_code=ErrorCode.FORBIDDEN, + error_category=ErrorCategory.AUTH, + detail="Forbidden", + retryable=False, ), - ( - 404, - "NotFound", - "Requested resource does not exist.", - ErrorCode.RECORD_NOT_FOUND, - ErrorCategory.NOT_FOUND, - "Resource not found", - False, + _ErrorResponseSpec( + status=404, + key="NotFound", + description="Requested resource does not exist.", + error_code=ErrorCode.RECORD_NOT_FOUND, + error_category=ErrorCategory.NOT_FOUND, + detail="Resource not found", + retryable=False, ), - ( - 409, - "Conflict", - "Resource conflict — duplicate or invalid state transition.", - ErrorCode.RESOURCE_CONFLICT, - ErrorCategory.CONFLICT, - "Resource conflict", - False, + _ErrorResponseSpec( + status=409, + key="Conflict", + description=("Resource conflict \u2014 duplicate or invalid state transition."), + error_code=ErrorCode.RESOURCE_CONFLICT, + error_category=ErrorCategory.CONFLICT, + detail="Resource conflict", + retryable=False, ), - ( - 429, - "TooManyRequests", - "Rate limit exceeded — back off and retry.", - ErrorCode.RATE_LIMITED, - ErrorCategory.RATE_LIMIT, - "Rate limit exceeded", - True, + _ErrorResponseSpec( + status=429, + key="TooManyRequests", + description="Rate limit exceeded \u2014 back off and retry.", + error_code=ErrorCode.RATE_LIMITED, + error_category=ErrorCategory.RATE_LIMIT, + detail="Rate limit exceeded", + retryable=True, ), - ( - 500, - "InternalError", - "Internal server error.", - ErrorCode.INTERNAL_ERROR, - ErrorCategory.INTERNAL, - "Internal server error", - False, + _ErrorResponseSpec( + status=500, + key="InternalError", + description="Internal server error.", + error_code=ErrorCode.INTERNAL_ERROR, + error_category=ErrorCategory.INTERNAL, + detail="Internal server error", + retryable=False, ), - ( - 503, - "ServiceUnavailable", - "Required service is temporarily unavailable.", - ErrorCode.SERVICE_UNAVAILABLE, - ErrorCategory.INTERNAL, - "Service unavailable", - True, + _ErrorResponseSpec( + status=503, + key="ServiceUnavailable", + description="Required service is temporarily unavailable.", + error_code=ErrorCode.SERVICE_UNAVAILABLE, + error_category=ErrorCategory.INTERNAL, + detail="Service unavailable", + retryable=True, ), ) -_INFO_DESCRIPTION: Final[str] = """\ -SynthOrg REST API for managing synthetic organizations \u2014 autonomous \ -AI agents orchestrated as a virtual company. - +_RFC9457_DESCRIPTION_SECTION: Final[str] = """\ ## Error Handling (RFC 9457) All error responses support content negotiation between two formats: @@ -163,6 +182,14 @@ def _build_problem_detail_schema() -> dict[str, Any]: ``#/components/schemas/`` so they resolve correctly when placed inside the OpenAPI ``components.schemas`` section. + .. note:: + + The ``$defs`` block is stripped because the referenced schemas + (e.g. ``ErrorCode``, ``ErrorCategory``) are already present in + the Litestar-generated ``components.schemas``. If this function + is used with a schema that lacks those definitions, the rewritten + ``$ref`` paths will be dangling. + Returns: JSON Schema dict for ``ProblemDetail`` with ``$defs`` stripped and ``$ref`` paths rewritten to resolve under @@ -170,7 +197,7 @@ def _build_problem_detail_schema() -> dict[str, Any]: """ raw = ProblemDetail.model_json_schema(mode="serialization") - # Strip $defs — they'll be merged separately. + # Strip $defs — referenced types already exist in components.schemas. raw.pop("$defs", None) # Rewrite $ref from '#/$defs/X' to '#/components/schemas/X'. @@ -179,12 +206,20 @@ def _build_problem_detail_schema() -> dict[str, Any]: def _rewrite_refs(obj: Any) -> Any: - """Recursively rewrite ``$ref`` paths from Pydantic to OpenAPI.""" + """Recursively rewrite ``$ref`` paths from Pydantic to OpenAPI. + + Only rewrites ``#/$defs/``-prefixed refs to + ``#/components/schemas/``. Other prefixes (e.g. + already-rewritten ``#/components/schemas/``) pass through + unchanged for idempotency. + """ if isinstance(obj, dict): if "$ref" in obj: ref: str = obj["$ref"] if ref.startswith("#/$defs/"): - return {"$ref": f"#/components/schemas/{ref.removeprefix('#/$defs/')}"} + return { + "$ref": (f"#/components/schemas/{ref.removeprefix('#/$defs/')}"), + } return {k: _rewrite_refs(v) for k, v in obj.items()} if isinstance(obj, list): return [_rewrite_refs(item) for item in obj] @@ -242,36 +277,28 @@ def _problem_detail_example( } -def _build_reusable_response( # noqa: PLR0913 - *, - status: int, - description: str, - error_code: ErrorCode, - error_category: ErrorCategory, - detail: str, - retryable: bool, -) -> dict[str, Any]: +def _build_reusable_response(spec: _ErrorResponseSpec) -> dict[str, Any]: """Build a reusable response object with dual content types.""" return { - "description": description, + "description": spec.description, "content": { _APP_JSON: { "schema": {"$ref": _ENVELOPE_REF}, "example": _envelope_example( - detail=detail, - error_code=error_code, - error_category=error_category, - retryable=retryable, + detail=spec.detail, + error_code=spec.error_code, + error_category=spec.error_category, + retryable=spec.retryable, ), }, _PROBLEM_JSON: { "schema": {"$ref": _PROBLEM_DETAIL_REF}, "example": _problem_detail_example( - status=status, - detail=detail, - error_code=error_code, - error_category=error_category, - retryable=retryable, + status=spec.status, + detail=spec.detail, + error_code=spec.error_code, + error_category=spec.error_category, + retryable=spec.retryable, ), }, }, @@ -295,9 +322,6 @@ def _response_ref(key: str) -> dict[str, str]: # ── Response-to-operation mapping ───────────────────────────── -# Maps response keys to the condition under which they are injected. -# Condition signature: (path: str, method: str, operation: dict) -> bool - def _should_inject( key: str, @@ -308,7 +332,8 @@ def _should_inject( """Decide whether to inject a response reference into an operation. Returns ``True`` when the given error response *key* is applicable - to the *path*/*method* combination. + to the *path*/*method* combination. Returns ``False`` for + unrecognised keys (defensive fallback). """ is_public = _is_public_path(path) is_write = method in _WRITE_METHODS @@ -322,9 +347,8 @@ def _should_inject( # Inject on write methods or replace Litestar's incorrect default. "BadRequest": is_write or "400" in operation.get("responses", {}), "NotFound": has_params, - # DELETE is excluded — it's idempotent and conflicts are - # a create/update concern in this API's model. - "Conflict": method in {"post", "put", "patch"}, + # DELETE excluded — idempotent; conflicts are a create/update concern. + "Conflict": is_write and method != "delete", "TooManyRequests": not is_public, } return checks.get(key, False) @@ -349,11 +373,45 @@ def _inject_operation_responses( status_code = status_for_key[key] if not _should_inject(key, path, method, operation): continue - # Always replace 400 (Litestar's default is incorrect). + # Replace Litestar's auto-generated 400 (ValidationException) + # with the RFC 9457 dual-format version. All 400 responses + # in Litestar-generated schemas use the default + # ValidationException schema; custom 400s would need manual + # post-processing. if status_code == "400" or status_code not in op_responses: op_responses[status_code] = _response_ref(key) +# ── Extracted steps ─────────────────────────────────────────── + + +def _add_problem_detail_schema(schemas: dict[str, Any]) -> None: + """Add ``ProblemDetail`` to ``components.schemas`` if absent.""" + if "ProblemDetail" not in schemas: + schemas["ProblemDetail"] = _build_problem_detail_schema() + + +def _build_all_responses( + responses: dict[str, Any], +) -> tuple[list[str], dict[str, str]]: + """Build reusable error responses and return keys + status mapping.""" + response_keys: list[str] = [] + status_for_key: dict[str, str] = {} + for spec in _ERROR_RESPONSES: + responses[spec.key] = _build_reusable_response(spec) + response_keys.append(spec.key) + status_for_key[spec.key] = str(spec.status) + return response_keys, status_for_key + + +def _update_info_description(info: dict[str, Any]) -> None: + """Append RFC 9457 documentation to ``info.description`` idempotently.""" + existing = info.get("description", "") + if "## Error Handling (RFC 9457)" not in existing: + separator = "\n\n" if existing else "" + info["description"] = f"{existing}{separator}{_RFC9457_DESCRIPTION_SECTION}" + + # ── Main function ───────────────────────────────────────────── @@ -367,7 +425,7 @@ def inject_rfc9457_responses(schema: dict[str, Any]) -> dict[str, Any]: - Reusable error responses (dual content types) in ``components.responses`` - Error response refs injected into every operation - - ``info.description`` updated with RFC 9457 documentation + - RFC 9457 docs appended to ``info.description`` Args: schema: OpenAPI schema dict (not modified). @@ -381,36 +439,13 @@ def inject_rfc9457_responses(schema: dict[str, Any]) -> dict[str, Any]: schemas = components.setdefault("schemas", {}) responses = components.setdefault("responses", {}) - # 1. Add ProblemDetail schema. - if "ProblemDetail" not in schemas: - schemas["ProblemDetail"] = _build_problem_detail_schema() - - # 2. Build reusable responses with dual content types. - response_keys: list[str] = [] - for ( - status, - key, - description, - error_code, - error_category, - detail, - retryable, - ) in _ERROR_RESPONSES: - responses[key] = _build_reusable_response( - status=status, - description=description, - error_code=error_code, - error_category=error_category, - detail=detail, - retryable=retryable, - ) - response_keys.append(key) - - # 3. Inject error response refs into individual operations. - status_for_key = {key: str(status) for status, key, *_ in _ERROR_RESPONSES} - _inject_operation_responses(result.get("paths", {}), response_keys, status_for_key) - - # 4. Update info.description. - result.setdefault("info", {})["description"] = _INFO_DESCRIPTION + _add_problem_detail_schema(schemas) + response_keys, status_for_key = _build_all_responses(responses) + _inject_operation_responses( + result.get("paths", {}), + response_keys, + status_for_key, + ) + _update_info_description(result.setdefault("info", {})) return result diff --git a/tests/integration/api/__init__.py b/tests/integration/api/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/integration/api/test_openapi_integration.py b/tests/integration/api/test_openapi_integration.py new file mode 100644 index 0000000000..f16dc49731 --- /dev/null +++ b/tests/integration/api/test_openapi_integration.py @@ -0,0 +1,63 @@ +"""Integration test for RFC 9457 OpenAPI schema post-processing. + +Exercises :func:`inject_rfc9457_responses` against the real +Litestar-generated schema end-to-end. +""" + +from typing import Any + +import pytest + +from synthorg.api.openapi import inject_rfc9457_responses + +_EXPECTED_RESPONSE_KEYS = frozenset( + { + "BadRequest", + "Unauthorized", + "Forbidden", + "NotFound", + "Conflict", + "TooManyRequests", + "InternalError", + "ServiceUnavailable", + } +) + + +@pytest.mark.integration +def test_full_app_schema_enhancement() -> None: + """Enhance the real Litestar-generated schema end-to-end.""" + from synthorg.api.app import create_app + + app = create_app() + schema: dict[str, Any] = app.openapi_schema.to_schema() + result = inject_rfc9457_responses(schema) + + # ProblemDetail schema present. + assert "ProblemDetail" in result["components"]["schemas"] + + # All 8 reusable responses defined (subset check — schema may + # contain additional non-RFC-9457 reusable responses). + responses = result["components"]["responses"] + assert _EXPECTED_RESPONSE_KEYS.issubset(responses.keys()) + + # Every RFC 9457 response has dual content types. + for key in _EXPECTED_RESPONSE_KEYS: + resp = responses[key] + content = resp["content"] + assert "application/json" in content, f"{key} missing application/json" + assert "application/problem+json" in content, f"{key} missing problem+json" + + # At least one operation has error response refs. + tasks_get = result["paths"]["/api/v1/tasks"]["get"]["responses"] + assert "500" in tasks_get + assert tasks_get["500"] == { + "$ref": "#/components/responses/InternalError", + } + + # Public endpoints don't have 401. + health = result["paths"]["/api/v1/health"]["get"]["responses"] + assert "401" not in health + + # Info description updated. + assert "RFC 9457" in result["info"]["description"] diff --git a/tests/unit/api/test_openapi.py b/tests/unit/api/test_openapi.py index e506d11ab1..c8e9988ae3 100644 --- a/tests/unit/api/test_openapi.py +++ b/tests/unit/api/test_openapi.py @@ -10,7 +10,7 @@ import pytest -from synthorg.api.openapi import inject_rfc9457_responses +from synthorg.api.openapi import _should_inject, inject_rfc9457_responses # ── Fixtures ────────────────────────────────────────────────── @@ -147,7 +147,9 @@ def test_reuses_existing_error_category_ref( result = inject_rfc9457_responses(base_schema) pd = result["components"]["schemas"]["ProblemDetail"] error_cat_prop = pd["properties"]["error_category"] - assert error_cat_prop == {"$ref": "#/components/schemas/ErrorCategory"} + assert error_cat_prop == { + "$ref": "#/components/schemas/ErrorCategory", + } def test_not_overwritten_if_exists(self) -> None: """Pre-existing ProblemDetail schema is preserved.""" @@ -156,6 +158,12 @@ def test_not_overwritten_if_exists(self) -> None: result = inject_rfc9457_responses(schema) assert result["components"]["schemas"]["ProblemDetail"]["custom"] is True + def test_no_defs_in_problem_detail(self, base_schema: dict[str, Any]) -> None: + """ProblemDetail schema has no leftover $defs.""" + result = inject_rfc9457_responses(base_schema) + pd = result["components"]["schemas"]["ProblemDetail"] + assert "$defs" not in pd + # ── Reusable responses ──────────────────────────────────────── @@ -172,6 +180,17 @@ def test_not_overwritten_if_exists(self) -> None: } ) +_EXPECTED_STATUS_CODES: dict[str, int] = { + "BadRequest": 400, + "Unauthorized": 401, + "Forbidden": 403, + "NotFound": 404, + "Conflict": 409, + "TooManyRequests": 429, + "InternalError": 500, + "ServiceUnavailable": 503, +} + @pytest.mark.unit class TestReusableResponses: @@ -211,7 +230,9 @@ def test_problem_json_refs_problem_detail( pj_content = result["components"]["responses"][key]["content"][ "application/problem+json" ] - assert pj_content["schema"] == {"$ref": "#/components/schemas/ProblemDetail"} + assert pj_content["schema"] == { + "$ref": "#/components/schemas/ProblemDetail", + } @pytest.mark.parametrize("key", sorted(_EXPECTED_RESPONSE_KEYS)) def test_examples_present(self, base_schema: dict[str, Any], key: str) -> None: @@ -255,6 +276,23 @@ def test_problem_detail_example_structure( assert "error_code" in example assert "error_category" in example + @pytest.mark.parametrize( + ("key", "expected_status"), + sorted(_EXPECTED_STATUS_CODES.items()), + ) + def test_problem_detail_example_status_matches_http_code( + self, + base_schema: dict[str, Any], + key: str, + expected_status: int, + ) -> None: + """ProblemDetail example status matches the HTTP status code.""" + result = inject_rfc9457_responses(base_schema) + example = result["components"]["responses"][key]["content"][ + "application/problem+json" + ]["example"] + assert example["status"] == expected_status + # ── Operation injection ─────────────────────────────────────── @@ -287,7 +325,11 @@ def test_authenticated_endpoints_have_401( def test_public_endpoints_skip_401(self, base_schema: dict[str, Any]) -> None: result = inject_rfc9457_responses(base_schema) - for path in ("/api/v1/health", "/api/v1/auth/login", "/api/v1/auth/setup"): + for path in ( + "/api/v1/health", + "/api/v1/auth/login", + "/api/v1/auth/setup", + ): for method in result["paths"][path]: op = result["paths"][path][method] if not isinstance(op, dict): @@ -298,7 +340,10 @@ def test_public_endpoints_skip_401(self, base_schema: dict[str, Any]) -> None: def test_path_param_endpoints_have_404(self, base_schema: dict[str, Any]) -> None: result = inject_rfc9457_responses(base_schema) - for path in ("/api/v1/tasks/{task_id}", "/api/v1/agents/{agent_name}"): + for path in ( + "/api/v1/tasks/{task_id}", + "/api/v1/agents/{agent_name}", + ): for method in result["paths"][path]: responses = result["paths"][path][method]["responses"] assert "404" in responses, f"{method.upper()} {path} missing 404" @@ -425,7 +470,8 @@ def test_retryable_example_has_retryable_true( assert problem_ex["retryable"] is True @pytest.mark.parametrize( - "key", ["BadRequest", "Unauthorized", "Forbidden", "NotFound", "Conflict"] + "key", + ["BadRequest", "Unauthorized", "Forbidden", "NotFound", "Conflict"], ) def test_non_retryable_example_has_retryable_false( self, base_schema: dict[str, Any], key: str @@ -437,6 +483,58 @@ def test_non_retryable_example_has_retryable_false( problem_ex = content["application/problem+json"]["example"] assert problem_ex["retryable"] is False + def test_put_endpoints_have_write_responses(self) -> None: + """PUT method gets 400, 403, and 409 injected.""" + schema = _minimal_schema( + paths={ + "/api/v1/tasks/{task_id}": { + "put": _minimal_operation(has_body=True), + }, + }, + ) + result = inject_rfc9457_responses(schema) + responses = result["paths"]["/api/v1/tasks/{task_id}"]["put"]["responses"] + assert "400" in responses + assert "403" in responses + assert "409" in responses + + def test_get_without_existing_400_skips_bad_request( + self, base_schema: dict[str, Any] + ) -> None: + """GET endpoint without pre-existing 400 does not get BadRequest.""" + result = inject_rfc9457_responses(base_schema) + # GET /tasks has no pre-existing 400 and is not a write method. + responses = result["paths"]["/api/v1/tasks"]["get"]["responses"] + assert "400" not in responses + + def test_skips_non_operation_entries(self) -> None: + """Path-level metadata (parameters) is not treated as operations.""" + schema = _minimal_schema( + paths={ + "/api/v1/items/{id}": { + "parameters": [{"name": "id", "in": "path"}], + "get": _minimal_operation(), + }, + }, + ) + result = inject_rfc9457_responses(schema) + # Should not crash; GET should have 500 injected. + responses = result["paths"]["/api/v1/items/{id}"]["get"]["responses"] + assert "500" in responses + # Parameters list should be unchanged. + params = result["paths"]["/api/v1/items/{id}"]["parameters"] + assert isinstance(params, list) + + def test_unknown_key_returns_false(self) -> None: + """Unknown response key falls back to False (not injected).""" + result = _should_inject( + key="UnknownResponse", + path="/api/v1/tasks", + method="get", + operation={"responses": {"200": {"description": "OK"}}}, + ) + assert result is False + # ── Info description ────────────────────────────────────────── @@ -461,6 +559,15 @@ def test_mentions_error_reference(self, base_schema: dict[str, Any]) -> None: desc = result["info"]["description"] assert "synthorg.io/docs/errors" in desc + def test_preserves_existing_description(self) -> None: + """Existing info.description is preserved with RFC section appended.""" + schema = _minimal_schema() + schema["info"]["description"] = "My custom API description." + result = inject_rfc9457_responses(schema) + desc = result["info"]["description"] + assert desc.startswith("My custom API description.") + assert "RFC 9457" in desc + # ── Idempotency and immutability ────────────────────────────── @@ -488,44 +595,9 @@ def test_empty_paths(self) -> None: def test_missing_components(self) -> None: """Handles schema with missing components section.""" - schema = {"openapi": "3.1.0", "info": {"title": "X", "version": "1"}} + schema = { + "openapi": "3.1.0", + "info": {"title": "X", "version": "1"}, + } result = inject_rfc9457_responses(schema) assert "ProblemDetail" in result["components"]["schemas"] - - -# ── Integration ─────────────────────────────────────────────── - - -@pytest.mark.integration -def test_full_app_schema_enhancement() -> None: - """Enhance the real Litestar-generated schema end-to-end.""" - from synthorg.api.app import create_app - - app = create_app() - schema = app.openapi_schema.to_schema() - result = inject_rfc9457_responses(schema) - - # ProblemDetail schema present. - assert "ProblemDetail" in result["components"]["schemas"] - - # All 8 reusable responses defined. - responses = result["components"]["responses"] - assert set(responses.keys()) == _EXPECTED_RESPONSE_KEYS - - # Every reusable response has dual content types. - for key, resp in responses.items(): - content = resp["content"] - assert "application/json" in content, f"{key} missing application/json" - assert "application/problem+json" in content, f"{key} missing problem+json" - - # At least one operation has error response refs. - tasks_get = result["paths"]["/api/v1/tasks"]["get"]["responses"] - assert "500" in tasks_get - assert tasks_get["500"] == {"$ref": "#/components/responses/InternalError"} - - # Public endpoints don't have 401. - health = result["paths"]["/api/v1/health"]["get"]["responses"] - assert "401" not in health - - # Info description updated. - assert "RFC 9457" in result["info"]["description"] From cce967aa2b312d53b9d746c9168ffab72bdcfb7b Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Tue, 17 Mar 2026 00:06:48 +0100 Subject: [PATCH 4/7] refactor: add debug logging to OpenAPI post-processor Add API_OPENAPI_SCHEMA_ENHANCED event constant and a debug log call in inject_rfc9457_responses to satisfy the logger-imported-but-unused finding from CodeRabbit's second review pass. --- CLAUDE.md | 2 +- src/synthorg/api/openapi.py | 8 ++++++++ src/synthorg/observability/events/api.py | 1 + 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 67bbdff2bf..f50b5ec59b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -196,7 +196,7 @@ site/ # Astro landing page (synthorg.io) - **Every module** with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)` - **Never** use `import logging` / `logging.getLogger()` / `print()` in application code - **Variable name**: always `logger` (not `_logger`, not `log`) -- **Event names**: always use constants from the domain-specific module under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`, `BUDGET_RECORD_ADDED` from `events.budget`, `CFO_ANOMALY_DETECTED` from `events.cfo`, `CONFLICT_DETECTED` from `events.conflict`, `MEETING_STARTED` from `events.meeting`, `MEETING_SCHEDULER_STARTED` from `events.meeting`, `MEETING_SCHEDULER_ERROR` from `events.meeting`, `MEETING_SCHEDULER_STOPPED` from `events.meeting`, `MEETING_PERIODIC_TRIGGERED` from `events.meeting`, `MEETING_EVENT_TRIGGERED` from `events.meeting`, `MEETING_PARTICIPANTS_RESOLVED` from `events.meeting`, `MEETING_NO_PARTICIPANTS` from `events.meeting`, `MEETING_NOT_FOUND` from `events.meeting`, `CLASSIFICATION_START` from `events.classification`, `CONSOLIDATION_START` from `events.consolidation`, `ORG_MEMORY_QUERY_START` from `events.org_memory`, `API_REQUEST_STARTED` from `events.api`, `API_REQUEST_COMPLETED` from `events.api`, `API_REQUEST_ERROR` from `events.api`, `API_ROUTE_NOT_FOUND` from `events.api`, `API_HEALTH_CHECK` from `events.api`, `API_COORDINATION_STARTED` from `events.api`, `API_COORDINATION_COMPLETED` from `events.api`, `API_COORDINATION_FAILED` from `events.api`, `API_COORDINATION_AGENT_RESOLVE_FAILED` from `events.api`, `API_CONTENT_NEGOTIATED` from `events.api`, `API_CORRELATION_FALLBACK` from `events.api`, `API_ACCEPT_PARSE_FAILED` from `events.api`, `API_WS_TICKET_ISSUED` from `events.api`, `API_WS_TICKET_CONSUMED` from `events.api`, `API_WS_TICKET_EXPIRED` from `events.api`, `API_WS_TICKET_INVALID` from `events.api`, `API_WS_TICKET_CLEANUP` from `events.api`, `CODE_RUNNER_EXECUTE_START` from `events.code_runner`, `DOCKER_EXECUTE_START` from `events.docker`, `MCP_INVOKE_START` from `events.mcp`, `SECURITY_EVALUATE_START` from `events.security`, `HR_HIRING_REQUEST_CREATED` from `events.hr`, `PERF_METRIC_RECORDED` from `events.performance`, `PERF_LLM_SAMPLE_STARTED` from `events.performance`, `PERF_LLM_SAMPLE_COMPLETED` from `events.performance`, `PERF_LLM_SAMPLE_FAILED` from `events.performance`, `PERF_OVERRIDE_SET` from `events.performance`, `PERF_OVERRIDE_CLEARED` from `events.performance`, `PERF_OVERRIDE_APPLIED` from `events.performance`, `PERF_OVERRIDE_EXPIRED` from `events.performance`, `TRUST_EVALUATE_START` from `events.trust`, `PROMOTION_EVALUATE_START` from `events.promotion`, `PROMPT_BUILD_START` from `events.prompt`, `MEMORY_RETRIEVAL_START` from `events.memory`, `MEMORY_BACKEND_CONNECTED` from `events.memory`, `MEMORY_ENTRY_STORED` from `events.memory`, `MEMORY_BACKEND_SYSTEM_ERROR` from `events.memory`, `MEMORY_RRF_FUSION_COMPLETE` from `events.memory`, `MEMORY_RRF_VALIDATION_FAILED` from `events.memory`, `AUTONOMY_ACTION_AUTO_APPROVED` from `events.autonomy`, `TIMEOUT_POLICY_EVALUATED` from `events.timeout`, `PERSISTENCE_AUDIT_ENTRY_SAVED` from `events.persistence`, `TASK_ENGINE_STARTED` from `events.task_engine`, `COORDINATION_STARTED` from `events.coordination`, `COORDINATION_FACTORY_BUILT` from `events.coordination`, `COMMUNICATION_DISPATCH_START` from `events.communication`, `COMPANY_STARTED` from `events.company`, `CONFIG_LOADED` from `events.config`, `CORRELATION_ID_CREATED` from `events.correlation`, `DECOMPOSITION_STARTED` from `events.decomposition`, `DELEGATION_STARTED` from `events.delegation`, `EXECUTION_LOOP_START` from `events.execution`, `CHECKPOINT_SAVED` from `events.checkpoint`, `PERSISTENCE_CHECKPOINT_SAVED` from `events.persistence`, `GIT_OPERATION_START` from `events.git`, `PARALLEL_GROUP_START` from `events.parallel`, `PERSONALITY_LOADED` from `events.personality`, `QUOTA_CHECKED` from `events.quota`, `ROLE_ASSIGNED` from `events.role`, `ROUTING_STARTED` from `events.routing`, `SANDBOX_EXECUTE_START` from `events.sandbox`, `TASK_CREATED` from `events.task`, `TASK_ASSIGNMENT_STARTED` from `events.task_assignment`, `TASK_ROUTING_STARTED` from `events.task_routing`, `TEMPLATE_LOADED` from `events.template`, `TOOL_INVOKE_START` from `events.tool`, `TOOL_OUTPUT_WITHHELD` from `events.tool`, `WORKSPACE_CREATED` from `events.workspace`, `APPROVAL_GATE_ESCALATION_DETECTED` from `events.approval_gate`, `APPROVAL_GATE_ESCALATION_FAILED` from `events.approval_gate`, `APPROVAL_GATE_INITIALIZED` from `events.approval_gate`, `APPROVAL_GATE_RISK_CLASSIFIED` from `events.approval_gate`, `APPROVAL_GATE_RISK_CLASSIFY_FAILED` from `events.approval_gate`, `APPROVAL_GATE_CONTEXT_PARKED` from `events.approval_gate`, `APPROVAL_GATE_CONTEXT_PARK_FAILED` from `events.approval_gate`, `APPROVAL_GATE_PARK_TASKLESS` from `events.approval_gate`, `APPROVAL_GATE_RESUME_STARTED` from `events.approval_gate`, `APPROVAL_GATE_CONTEXT_RESUMED` from `events.approval_gate`, `APPROVAL_GATE_RESUME_FAILED` from `events.approval_gate`, `APPROVAL_GATE_RESUME_DELETE_FAILED` from `events.approval_gate`, `APPROVAL_GATE_RESUME_TRIGGERED` from `events.approval_gate`, `APPROVAL_GATE_NO_PARKED_CONTEXT` from `events.approval_gate`, `APPROVAL_GATE_LOOP_WIRING_WARNING` from `events.approval_gate`, `STAGNATION_CHECK_PERFORMED` from `events.stagnation`, `STAGNATION_DETECTED` from `events.stagnation`, `STAGNATION_CORRECTION_INJECTED` from `events.stagnation`, `STAGNATION_TERMINATED` from `events.stagnation`, `PERSISTENCE_AGENT_STATE_SAVED` from `events.persistence`, `PERSISTENCE_AGENT_STATE_FETCHED` from `events.persistence`, `PERSISTENCE_AGENT_STATE_ACTIVE_QUERIED` from `events.persistence`, `PERSISTENCE_AGENT_STATE_DELETED` from `events.persistence`, `SETTINGS_VALUE_SET` from `events.settings`, `SETTINGS_VALUE_DELETED` from `events.settings`, `SETTINGS_VALUE_RESOLVED` from `events.settings`, `SETTINGS_CACHE_INVALIDATED` from `events.settings`, `SETTINGS_ENCRYPTION_ERROR` from `events.settings`, `SETTINGS_VALIDATION_FAILED` from `events.settings`, `SETTINGS_NOTIFICATION_PUBLISHED` from `events.settings`, `SETTINGS_NOTIFICATION_FAILED` from `events.settings`, `SETTINGS_FETCH_FAILED` from `events.settings`, `SETTINGS_SET_FAILED` from `events.settings`, `SETTINGS_DELETE_FAILED` from `events.settings`, `SETTINGS_NOT_FOUND` from `events.settings`, `SETTINGS_REGISTRY_DUPLICATE` from `events.settings`, `SETTINGS_CONFIG_PATH_MISS` from `events.settings`). Import directly: `from synthorg.observability.events. import EVENT_CONSTANT` +- **Event names**: always use constants from the domain-specific module under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`, `BUDGET_RECORD_ADDED` from `events.budget`, `CFO_ANOMALY_DETECTED` from `events.cfo`, `CONFLICT_DETECTED` from `events.conflict`, `MEETING_STARTED` from `events.meeting`, `MEETING_SCHEDULER_STARTED` from `events.meeting`, `MEETING_SCHEDULER_ERROR` from `events.meeting`, `MEETING_SCHEDULER_STOPPED` from `events.meeting`, `MEETING_PERIODIC_TRIGGERED` from `events.meeting`, `MEETING_EVENT_TRIGGERED` from `events.meeting`, `MEETING_PARTICIPANTS_RESOLVED` from `events.meeting`, `MEETING_NO_PARTICIPANTS` from `events.meeting`, `MEETING_NOT_FOUND` from `events.meeting`, `CLASSIFICATION_START` from `events.classification`, `CONSOLIDATION_START` from `events.consolidation`, `ORG_MEMORY_QUERY_START` from `events.org_memory`, `API_REQUEST_STARTED` from `events.api`, `API_REQUEST_COMPLETED` from `events.api`, `API_REQUEST_ERROR` from `events.api`, `API_ROUTE_NOT_FOUND` from `events.api`, `API_HEALTH_CHECK` from `events.api`, `API_COORDINATION_STARTED` from `events.api`, `API_COORDINATION_COMPLETED` from `events.api`, `API_COORDINATION_FAILED` from `events.api`, `API_COORDINATION_AGENT_RESOLVE_FAILED` from `events.api`, `API_CONTENT_NEGOTIATED` from `events.api`, `API_CORRELATION_FALLBACK` from `events.api`, `API_ACCEPT_PARSE_FAILED` from `events.api`, `API_WS_TICKET_ISSUED` from `events.api`, `API_WS_TICKET_CONSUMED` from `events.api`, `API_WS_TICKET_EXPIRED` from `events.api`, `API_WS_TICKET_INVALID` from `events.api`, `API_WS_TICKET_CLEANUP` from `events.api`, `API_OPENAPI_SCHEMA_ENHANCED` from `events.api`, `CODE_RUNNER_EXECUTE_START` from `events.code_runner`, `DOCKER_EXECUTE_START` from `events.docker`, `MCP_INVOKE_START` from `events.mcp`, `SECURITY_EVALUATE_START` from `events.security`, `HR_HIRING_REQUEST_CREATED` from `events.hr`, `PERF_METRIC_RECORDED` from `events.performance`, `PERF_LLM_SAMPLE_STARTED` from `events.performance`, `PERF_LLM_SAMPLE_COMPLETED` from `events.performance`, `PERF_LLM_SAMPLE_FAILED` from `events.performance`, `PERF_OVERRIDE_SET` from `events.performance`, `PERF_OVERRIDE_CLEARED` from `events.performance`, `PERF_OVERRIDE_APPLIED` from `events.performance`, `PERF_OVERRIDE_EXPIRED` from `events.performance`, `TRUST_EVALUATE_START` from `events.trust`, `PROMOTION_EVALUATE_START` from `events.promotion`, `PROMPT_BUILD_START` from `events.prompt`, `MEMORY_RETRIEVAL_START` from `events.memory`, `MEMORY_BACKEND_CONNECTED` from `events.memory`, `MEMORY_ENTRY_STORED` from `events.memory`, `MEMORY_BACKEND_SYSTEM_ERROR` from `events.memory`, `MEMORY_RRF_FUSION_COMPLETE` from `events.memory`, `MEMORY_RRF_VALIDATION_FAILED` from `events.memory`, `AUTONOMY_ACTION_AUTO_APPROVED` from `events.autonomy`, `TIMEOUT_POLICY_EVALUATED` from `events.timeout`, `PERSISTENCE_AUDIT_ENTRY_SAVED` from `events.persistence`, `TASK_ENGINE_STARTED` from `events.task_engine`, `COORDINATION_STARTED` from `events.coordination`, `COORDINATION_FACTORY_BUILT` from `events.coordination`, `COMMUNICATION_DISPATCH_START` from `events.communication`, `COMPANY_STARTED` from `events.company`, `CONFIG_LOADED` from `events.config`, `CORRELATION_ID_CREATED` from `events.correlation`, `DECOMPOSITION_STARTED` from `events.decomposition`, `DELEGATION_STARTED` from `events.delegation`, `EXECUTION_LOOP_START` from `events.execution`, `CHECKPOINT_SAVED` from `events.checkpoint`, `PERSISTENCE_CHECKPOINT_SAVED` from `events.persistence`, `GIT_OPERATION_START` from `events.git`, `PARALLEL_GROUP_START` from `events.parallel`, `PERSONALITY_LOADED` from `events.personality`, `QUOTA_CHECKED` from `events.quota`, `ROLE_ASSIGNED` from `events.role`, `ROUTING_STARTED` from `events.routing`, `SANDBOX_EXECUTE_START` from `events.sandbox`, `TASK_CREATED` from `events.task`, `TASK_ASSIGNMENT_STARTED` from `events.task_assignment`, `TASK_ROUTING_STARTED` from `events.task_routing`, `TEMPLATE_LOADED` from `events.template`, `TOOL_INVOKE_START` from `events.tool`, `TOOL_OUTPUT_WITHHELD` from `events.tool`, `WORKSPACE_CREATED` from `events.workspace`, `APPROVAL_GATE_ESCALATION_DETECTED` from `events.approval_gate`, `APPROVAL_GATE_ESCALATION_FAILED` from `events.approval_gate`, `APPROVAL_GATE_INITIALIZED` from `events.approval_gate`, `APPROVAL_GATE_RISK_CLASSIFIED` from `events.approval_gate`, `APPROVAL_GATE_RISK_CLASSIFY_FAILED` from `events.approval_gate`, `APPROVAL_GATE_CONTEXT_PARKED` from `events.approval_gate`, `APPROVAL_GATE_CONTEXT_PARK_FAILED` from `events.approval_gate`, `APPROVAL_GATE_PARK_TASKLESS` from `events.approval_gate`, `APPROVAL_GATE_RESUME_STARTED` from `events.approval_gate`, `APPROVAL_GATE_CONTEXT_RESUMED` from `events.approval_gate`, `APPROVAL_GATE_RESUME_FAILED` from `events.approval_gate`, `APPROVAL_GATE_RESUME_DELETE_FAILED` from `events.approval_gate`, `APPROVAL_GATE_RESUME_TRIGGERED` from `events.approval_gate`, `APPROVAL_GATE_NO_PARKED_CONTEXT` from `events.approval_gate`, `APPROVAL_GATE_LOOP_WIRING_WARNING` from `events.approval_gate`, `STAGNATION_CHECK_PERFORMED` from `events.stagnation`, `STAGNATION_DETECTED` from `events.stagnation`, `STAGNATION_CORRECTION_INJECTED` from `events.stagnation`, `STAGNATION_TERMINATED` from `events.stagnation`, `PERSISTENCE_AGENT_STATE_SAVED` from `events.persistence`, `PERSISTENCE_AGENT_STATE_FETCHED` from `events.persistence`, `PERSISTENCE_AGENT_STATE_ACTIVE_QUERIED` from `events.persistence`, `PERSISTENCE_AGENT_STATE_DELETED` from `events.persistence`, `SETTINGS_VALUE_SET` from `events.settings`, `SETTINGS_VALUE_DELETED` from `events.settings`, `SETTINGS_VALUE_RESOLVED` from `events.settings`, `SETTINGS_CACHE_INVALIDATED` from `events.settings`, `SETTINGS_ENCRYPTION_ERROR` from `events.settings`, `SETTINGS_VALIDATION_FAILED` from `events.settings`, `SETTINGS_NOTIFICATION_PUBLISHED` from `events.settings`, `SETTINGS_NOTIFICATION_FAILED` from `events.settings`, `SETTINGS_FETCH_FAILED` from `events.settings`, `SETTINGS_SET_FAILED` from `events.settings`, `SETTINGS_DELETE_FAILED` from `events.settings`, `SETTINGS_NOT_FOUND` from `events.settings`, `SETTINGS_REGISTRY_DUPLICATE` from `events.settings`, `SETTINGS_CONFIG_PATH_MISS` from `events.settings`). Import directly: `from synthorg.observability.events. import EVENT_CONSTANT` - **Structured kwargs**: always `logger.info(EVENT, key=value)` — never `logger.info("msg %s", val)` - **All error paths** must log at WARNING or ERROR with context before raising - **All state transitions** must log at INFO diff --git a/src/synthorg/api/openapi.py b/src/synthorg/api/openapi.py index 2e0133caa3..5aa7eea538 100644 --- a/src/synthorg/api/openapi.py +++ b/src/synthorg/api/openapi.py @@ -35,6 +35,7 @@ category_type_uri, ) from synthorg.observability import get_logger +from synthorg.observability.events.api import API_OPENAPI_SCHEMA_ENHANCED logger = get_logger(__name__) @@ -448,4 +449,11 @@ def inject_rfc9457_responses(schema: dict[str, Any]) -> dict[str, Any]: ) _update_info_description(result.setdefault("info", {})) + path_count = len(result.get("paths", {})) + logger.debug( + API_OPENAPI_SCHEMA_ENHANCED, + paths_processed=path_count, + responses_added=len(response_keys), + ) + return result diff --git a/src/synthorg/observability/events/api.py b/src/synthorg/observability/events/api.py index 8dc23330d9..ad13e434cb 100644 --- a/src/synthorg/observability/events/api.py +++ b/src/synthorg/observability/events/api.py @@ -55,3 +55,4 @@ API_WS_TICKET_EXPIRED: Final[str] = "api.ws.ticket_expired" API_WS_TICKET_INVALID: Final[str] = "api.ws.ticket_invalid" API_WS_TICKET_CLEANUP: Final[str] = "api.ws.ticket_cleanup" +API_OPENAPI_SCHEMA_ENHANCED: Final[str] = "api.openapi.schema_enhanced" From ed89b161dc7b500a1b6de58da260806f6b4ecd67 Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Tue, 17 Mar 2026 07:08:36 +0100 Subject: [PATCH 5/7] refactor: detect Litestar ValidationException before replacing 400 responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add _is_litestar_validation_400() heuristic that checks for the "Validation Exception" schema description — custom 400 responses are now preserved instead of unconditionally replaced - Add debug logging to _add_problem_detail_schema for schema transformation visibility - Add test_custom_400_preserved to verify custom 400s survive injection --- src/synthorg/api/openapi.py | 41 +++++++++++++++++++++++++++++----- tests/unit/api/test_openapi.py | 37 ++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 6 deletions(-) diff --git a/src/synthorg/api/openapi.py b/src/synthorg/api/openapi.py index 5aa7eea538..fd7bde6421 100644 --- a/src/synthorg/api/openapi.py +++ b/src/synthorg/api/openapi.py @@ -355,6 +355,20 @@ def _should_inject( return checks.get(key, False) +def _is_litestar_validation_400(response: dict[str, Any]) -> bool: + """Detect Litestar's auto-generated ``ValidationException`` 400 response. + + Returns ``True`` when the response schema contains the + ``"Validation Exception"`` description that Litestar emits for + request-body validation errors. Custom 400 responses will not + match this heuristic and are left untouched. + """ + content = response.get("content", {}) + json_content = content.get("application/json", {}) + schema = json_content.get("schema", {}) + return schema.get("description") == "Validation Exception" + + def _inject_operation_responses( paths: dict[str, Any], response_keys: list[str], @@ -374,12 +388,15 @@ def _inject_operation_responses( status_code = status_for_key[key] if not _should_inject(key, path, method, operation): continue - # Replace Litestar's auto-generated 400 (ValidationException) - # with the RFC 9457 dual-format version. All 400 responses - # in Litestar-generated schemas use the default - # ValidationException schema; custom 400s would need manual - # post-processing. - if status_code == "400" or status_code not in op_responses: + if status_code == "400": + # Only replace Litestar's auto-generated + # ValidationException 400; preserve custom 400s. + existing = op_responses.get("400") + if existing is None or _is_litestar_validation_400( + existing, + ): + op_responses["400"] = _response_ref(key) + elif status_code not in op_responses: op_responses[status_code] = _response_ref(key) @@ -390,6 +407,18 @@ def _add_problem_detail_schema(schemas: dict[str, Any]) -> None: """Add ``ProblemDetail`` to ``components.schemas`` if absent.""" if "ProblemDetail" not in schemas: schemas["ProblemDetail"] = _build_problem_detail_schema() + logger.debug( + API_OPENAPI_SCHEMA_ENHANCED, + step="add_problem_detail", + added=True, + ) + else: + logger.debug( + API_OPENAPI_SCHEMA_ENHANCED, + step="add_problem_detail", + added=False, + reason="already_exists", + ) def _build_all_responses( diff --git a/tests/unit/api/test_openapi.py b/tests/unit/api/test_openapi.py index c8e9988ae3..6b64a1f801 100644 --- a/tests/unit/api/test_openapi.py +++ b/tests/unit/api/test_openapi.py @@ -437,6 +437,43 @@ def test_default_400_replaced(self) -> None: # Should be replaced with our ref. assert resp_400 == {"$ref": "#/components/responses/BadRequest"} + def test_custom_400_preserved(self) -> None: + """Custom (non-Litestar) 400 response is not replaced.""" + custom_400 = { + "description": "Custom validation", + "content": { + "application/json": { + "schema": { + "type": "object", + "description": "Custom error schema", + }, + }, + }, + } + schema = _minimal_schema( + paths={ + "/api/v1/tasks": { + "post": { + "responses": { + "201": {"description": "Created"}, + "400": custom_400, + }, + "requestBody": { + "content": { + "application/json": { + "schema": {"type": "object"}, + }, + }, + }, + }, + }, + }, + ) + result = inject_rfc9457_responses(schema) + resp_400 = result["paths"]["/api/v1/tasks"]["post"]["responses"]["400"] + # Custom 400 should be preserved (not Litestar's ValidationException). + assert resp_400["description"] == "Custom validation" + def test_non_public_have_429(self, base_schema: dict[str, Any]) -> None: result = inject_rfc9457_responses(base_schema) responses = result["paths"]["/api/v1/tasks"]["get"]["responses"] From 39ee6af835c6e2ec8eb78b17a4493be4120bca38 Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Tue, 17 Mar 2026 07:13:05 +0100 Subject: [PATCH 6/7] fix: add type annotations to fix mypy strict no-any-return in _is_litestar_validation_400 --- src/synthorg/api/openapi.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/synthorg/api/openapi.py b/src/synthorg/api/openapi.py index fd7bde6421..24df37d8e8 100644 --- a/src/synthorg/api/openapi.py +++ b/src/synthorg/api/openapi.py @@ -363,10 +363,10 @@ def _is_litestar_validation_400(response: dict[str, Any]) -> bool: request-body validation errors. Custom 400 responses will not match this heuristic and are left untouched. """ - content = response.get("content", {}) - json_content = content.get("application/json", {}) - schema = json_content.get("schema", {}) - return schema.get("description") == "Validation Exception" + content: dict[str, Any] = response.get("content", {}) + json_content: dict[str, Any] = content.get("application/json", {}) + schema: dict[str, Any] = json_content.get("schema", {}) + return str(schema.get("description", "")) == "Validation Exception" def _inject_operation_responses( From f25bf7fb3cbee653564ce383694d4db951e5d105 Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Tue, 17 Mar 2026 07:22:03 +0100 Subject: [PATCH 7/7] refactor: clarify _WRITE_METHODS comment and add InternalError to non-retryable test - Update _WRITE_METHODS comment to explain DELETE inclusion for 400/403 and its intentional exclusion from 409 injection - Add InternalError to test_non_retryable_example_has_retryable_false parametrize list (retryable=False was untested for this response) --- src/synthorg/api/openapi.py | 3 ++- tests/unit/api/test_openapi.py | 9 ++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/synthorg/api/openapi.py b/src/synthorg/api/openapi.py index 24df37d8e8..e294eca458 100644 --- a/src/synthorg/api/openapi.py +++ b/src/synthorg/api/openapi.py @@ -51,7 +51,8 @@ "/auth/login", ) -# HTTP methods that accept a request body (get 400/409 injected). +# HTTP methods that mutate state. Includes DELETE for 400/403 injection; +# DELETE is intentionally excluded from 409 by the Conflict injection logic. _WRITE_METHODS: Final[frozenset[str]] = frozenset( {"post", "put", "patch", "delete"}, ) diff --git a/tests/unit/api/test_openapi.py b/tests/unit/api/test_openapi.py index 6b64a1f801..6faf66d8c6 100644 --- a/tests/unit/api/test_openapi.py +++ b/tests/unit/api/test_openapi.py @@ -508,7 +508,14 @@ def test_retryable_example_has_retryable_true( @pytest.mark.parametrize( "key", - ["BadRequest", "Unauthorized", "Forbidden", "NotFound", "Conflict"], + [ + "BadRequest", + "Unauthorized", + "Forbidden", + "NotFound", + "Conflict", + "InternalError", + ], ) def test_non_retryable_example_has_retryable_false( self, base_schema: dict[str, Any], key: str