From 245e6014a1e9f2bd64bba340e2adce4934c1b8e8 Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Wed, 15 Jul 2026 08:00:23 +0000 Subject: [PATCH 1/4] fix(api): return the REQ-010 error envelope on every error path (DEBT-033) Endpoints that raised HTTPException with a plain string detail returned FastAPI's bare {"detail": "..."} - no code, no remediation, no request id, so a client could not branch on an error code. The DOCS-009 caveat named four such endpoints; a grep of the handlers found four more (POST /query, the admin auth dependency, api-key creation, and admin /health?detail=full), so removing the caveat as written would have re-asserted a universality those still broke. All of them now raise ErrorResponse.to_envelope() at the same HTTP status they returned before (shape-only; no test asserted the bare shape). Reused ERR-CONFIG-001/002 for internal registry/key-store faults; added ERR-CONV-001/002/003, ERR-QUERY-005, ERR-ADMIN-008 and ERR-INDEX-003/004, each registered in errors.py, _CODE_STATUS_OVERRIDE and error-codes.md. Removed the "not every endpoint uses the envelope" caveat from api.md. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/reference/api.md | 17 ++--- docs/reference/error-codes.md | 7 ++ vektra-admin/src/vektra_admin/api.py | 53 +++++++++++---- vektra-admin/tests/test_admin_turns.py | 2 + vektra-core/src/vektra_core/api.py | 39 +++++++---- vektra-core/tests/test_feedback_api.py | 3 + vektra-index/src/vektra_index/reindex.py | 47 ++++++++++++-- vektra-index/tests/test_reindex.py | 21 ++++++ vektra-shared/src/vektra_shared/errors.py | 75 +++++++++++++++++++++ vektra-shared/tests/test_errors.py | 79 +++++++++++++++++++++++ 10 files changed, 302 insertions(+), 41 deletions(-) diff --git a/docs/reference/api.md b/docs/reference/api.md index 7da8e342..b20b962d 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -977,12 +977,14 @@ Response (HTTP 202): The source version is not a parameter: the job always reads the active version, since that is the only version the store exposes for reading. -Errors: `400` if `target_index_version` equals the active version (reindexing a version onto itself would overwrite the live chunks instead of writing beside them). +Errors: `400` `ERR-INDEX-003` if `target_index_version` equals the active version (reindexing a version onto itself would overwrite the live chunks instead of writing beside them). ### GET /api/v1/reindex/{job_id}/status Poll a reindex job. +Errors: `404` `ERR-INDEX-004` if no job with that id is visible to the key. + **Scopes**: `admin` ```bash @@ -1315,7 +1317,7 @@ curl -s http://localhost:8000/metrics ## Error responses -Most errors follow a standard envelope (REQ-010): +Every endpoint returns errors in the same envelope (REQ-010): ```json { @@ -1330,12 +1332,5 @@ Most errors follow a standard envelope (REQ-010): } ``` -See [error codes reference](error-codes.md) for the complete list. - -Not every endpoint uses the envelope. `POST /api/v1/reindex`, `GET /api/v1/reindex/{job_id}/status`, conversations and admin conversation turns return FastAPI's bare shape instead: - -```json -{"detail": "Reindex job not found"} -``` - -Clients that parse errors must handle both. The envelope is the intended contract; the bare form is a gap (DEBT-033), not a second contract to rely on. `DELETE /api/v1/index-versions/{version}` uses the envelope throughout, including its `409`. +Clients can branch on `error.code`, which is stable, rather than parsing +`message`. See [error codes reference](error-codes.md) for the complete list. diff --git a/docs/reference/error-codes.md b/docs/reference/error-codes.md index 3b5bd6a2..df2f727e 100644 --- a/docs/reference/error-codes.md +++ b/docs/reference/error-codes.md @@ -92,8 +92,15 @@ These codes are used by specific components and are not part of the REQ-011 regi | ERR-LEARN-004 | vektra-learn | 409 | Duplicate enrollment (student already enrolled in course) | | ERR-LEARN-005 | vektra-learn | 404 | Conversation not found (GET conversations/turns) | | ERR-LEARN-006 | vektra-learn | 403 | Conversation belongs to a different course/namespace | +| ERR-CONV-001 | vektra-core / vektra-admin | 404 | Conversation not found (core `GET`/`DELETE conversations/{id}`, admin turns) | +| ERR-CONV-002 | vektra-core / vektra-admin | 503 | Persistent conversation store not available (starting up, or none configured) | +| ERR-CONV-003 | vektra-admin | 501 | Configured conversation store cannot return decrypted turns (in-memory store) | +| ERR-QUERY-005 | vektra-core | 503 | Query pipeline provider not available (starting up, or none configured) | +| ERR-ADMIN-008 | vektra-admin | 503 | Service initializing (deep health check auth: registry/key store not ready) | | ERR-INDEX-001 | vektra-index | 409 | Refused: the index version named for deletion is the one currently being served. Nothing was deleted (REQ-064) | | ERR-INDEX-002 | vektra-index | 400 | Invalid index version (below 1) | +| ERR-INDEX-003 | vektra-index | 400 | Reindex target must differ from the active version | +| ERR-INDEX-004 | vektra-index | 404 | Reindex job not found | ## Adding new error codes diff --git a/vektra-admin/src/vektra_admin/api.py b/vektra-admin/src/vektra_admin/api.py index a98caa24..b354b673 100644 --- a/vektra-admin/src/vektra_admin/api.py +++ b/vektra-admin/src/vektra_admin/api.py @@ -38,10 +38,15 @@ from vektra_shared.auth import ApiKeyInfo, KeyStoreProvider, require_scope from vektra_shared.db import get_session from vektra_shared.errors import ( + ERR_CONFIG_002, ErrorCategory, ErrorResponse, auth_invalid_token, + conversation_not_found, + conversation_store_unavailable, + conversation_turns_unsupported, http_status_for, + provider_registry_unavailable, ) # --------------------------------------------------------------------------- @@ -111,12 +116,22 @@ async def _require_any_token( token = credentials.credentials registry = getattr(request.app.state, "registry", None) if registry is None: - raise HTTPException(status_code=500, detail="ProviderRegistry not initialized") + err = provider_registry_unavailable() + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) try: key_store: KeyStoreProvider = registry.get("key_store", "default") except ValueError: - raise HTTPException(status_code=500, detail="Key store not configured") + err = ErrorResponse( + category=ErrorCategory.CONFIGURATION, + code=ERR_CONFIG_002, + message="The API key store is not configured.", + remediation=( + "Verify the key store provider is registered. Check the server " + "logs and restart the service." + ), + ) + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) info = await key_store.lookup_by_token(token) if info is None: @@ -199,7 +214,13 @@ async def health( ) # Validate token; fail closed if registry/key_store unavailable if registry is None: - raise HTTPException(status_code=503, detail="Service initializing") + err = ErrorResponse( + category=ErrorCategory.TRANSIENT, + code="ERR-ADMIN-008", + message="The service is still initializing.", + remediation="Retry shortly; the service is starting up.", + ) + raise HTTPException(status_code=503, detail=err.to_envelope()) try: key_store = registry.get("key_store", "default") info = await key_store.lookup_by_token(credentials.credentials) @@ -211,7 +232,13 @@ async def health( # Expose key_id for AuditMiddleware (CR55) request.state.key_id = info.key_id except ValueError: - raise HTTPException(status_code=503, detail="Service initializing") + err = ErrorResponse( + category=ErrorCategory.TRANSIENT, + code="ERR-ADMIN-008", + message="The service is still initializing.", + remediation="Retry shortly; the service is starting up.", + ) + raise HTTPException(status_code=503, detail=err.to_envelope()) status_code = 503 if deep.status == "unhealthy" else 200 return Response( @@ -290,8 +317,9 @@ async def create_api_key( else: # Must be a valid admin-scoped key if registry is None: + err = provider_registry_unavailable() raise HTTPException( - status_code=500, detail="ProviderRegistry not initialized" + status_code=http_status_for(err), detail=err.to_envelope() ) try: key_store = registry.get("key_store", "default") @@ -516,22 +544,23 @@ async def get_conversation_turns( """Return decrypted conversation turns with full metadata (admin only).""" registry = getattr(request.app.state, "registry", None) if registry is None: - raise HTTPException(status_code=500, detail="ProviderRegistry not initialized") + err = provider_registry_unavailable() + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) try: conv_store = registry.get("conversation_store", "default") except ValueError: - raise HTTPException(status_code=503, detail="Conversation store not available") + err = conversation_store_unavailable() + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) if not hasattr(conv_store, "get_turns_detail"): - raise HTTPException( - status_code=501, - detail="Conversation decryption not available (in-memory store)", - ) + err = conversation_turns_unsupported() + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) turns = await conv_store.get_turns_detail(conversation_id) if turns is None: - raise HTTPException(status_code=404, detail="Conversation not found") + err = conversation_not_found(conversation_id) + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) # Audit log: sensitive content access request_id = _resolve_request_id(request) diff --git a/vektra-admin/tests/test_admin_turns.py b/vektra-admin/tests/test_admin_turns.py index a87f7b6d..c479edf6 100644 --- a/vektra-admin/tests/test_admin_turns.py +++ b/vektra-admin/tests/test_admin_turns.py @@ -77,6 +77,7 @@ async def test_returns_404_when_conversation_not_found(): with pytest.raises(HTTPException) as exc_info: await get_conversation_turns(uuid4(), request, MagicMock(), key_info) assert exc_info.value.status_code == 404 + assert exc_info.value.detail["error"]["code"] == "ERR-CONV-001" @pytest.mark.asyncio @@ -166,3 +167,4 @@ async def test_returns_501_for_inmemory_store(): with pytest.raises(HTTPException) as exc_info: await get_conversation_turns(uuid4(), request, MagicMock(), key_info) assert exc_info.value.status_code == 501 + assert exc_info.value.detail["error"]["code"] == "ERR-CONV-003" diff --git a/vektra-core/src/vektra_core/api.py b/vektra-core/src/vektra_core/api.py index cd6fad6a..05d90764 100644 --- a/vektra-core/src/vektra_core/api.py +++ b/vektra-core/src/vektra_core/api.py @@ -34,9 +34,13 @@ from vektra_shared.errors import ( ERR_QUERY_002, ERR_QUERY_003, + ERR_QUERY_005, ErrorCategory, ErrorResponse, + conversation_not_found, + conversation_store_unavailable, http_status_for, + provider_registry_unavailable, ) from vektra_shared.namespace import resolve_citations_enabled, resolve_grounding_mode from vektra_shared.types import ( @@ -204,12 +208,22 @@ async def query( registry = getattr(request.app.state, "registry", None) if registry is None: - raise HTTPException(status_code=500, detail="ProviderRegistry not initialized") + err = provider_registry_unavailable() + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) try: pipeline = registry.get("query_pipeline", "default") except ValueError: - raise HTTPException(status_code=503, detail="Query pipeline not configured") + err = ErrorResponse( + category=ErrorCategory.TRANSIENT, + code=ERR_QUERY_005, + message="The query pipeline is not available.", + remediation=( + "The service may be starting up, or no query pipeline is " + "configured. Retry shortly." + ), + ) + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) # Safeguard pre_query (input validation trust boundary, REQ-044) try: @@ -411,21 +425,18 @@ def _get_conversation_store(request: Request) -> PersistentConversationStore: """ registry = getattr(request.app.state, "registry", None) if registry is None: - raise HTTPException(status_code=500, detail="ProviderRegistry not initialized") + err = provider_registry_unavailable() + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) try: store = registry.get("conversation_store", "default") except ValueError: - raise HTTPException( - status_code=503, - detail="Persistent conversation storage is not configured", - ) + err = conversation_store_unavailable() + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) if not isinstance(store, PersistentConversationStore): - raise HTTPException( - status_code=503, - detail="Persistent conversation storage is not configured", - ) + err = conversation_store_unavailable() + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) return store @@ -443,7 +454,8 @@ async def get_conversation( store = _get_conversation_store(request) meta = await store.get_metadata(conversation_id, namespace=_key.namespace_id) if meta is None or meta.get("deleted_at") is not None: - raise HTTPException(status_code=404, detail="Conversation not found") + err = conversation_not_found(conversation_id) + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) return ConversationMetadata( id=meta["id"], @@ -468,7 +480,8 @@ async def delete_conversation( store = _get_conversation_store(request) deleted = await store.soft_delete(conversation_id, namespace=_key.namespace_id) if not deleted: - raise HTTPException(status_code=404, detail="Conversation not found") + err = conversation_not_found(conversation_id) + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) log.info( "conversation_deleted", diff --git a/vektra-core/tests/test_feedback_api.py b/vektra-core/tests/test_feedback_api.py index ab77b349..ce958068 100644 --- a/vektra-core/tests/test_feedback_api.py +++ b/vektra-core/tests/test_feedback_api.py @@ -295,6 +295,7 @@ async def test_get_conversation_not_found(): ) assert resp.status_code == 404 + assert resp.json()["detail"]["error"]["code"] == "ERR-CONV-001" async def test_get_conversation_soft_deleted_returns_404(): @@ -364,6 +365,7 @@ async def test_delete_conversation_not_found(): ) assert resp.status_code == 404 + assert resp.json()["detail"]["error"]["code"] == "ERR-CONV-001" async def test_conversation_no_persistent_store_returns_503(): @@ -381,6 +383,7 @@ async def test_conversation_no_persistent_store_returns_503(): ) assert resp.status_code == 503 + assert resp.json()["detail"]["error"]["code"] == "ERR-CONV-002" # --------------------------------------------------------------------------- diff --git a/vektra-index/src/vektra_index/reindex.py b/vektra-index/src/vektra_index/reindex.py index 38fc22ca..68e25a61 100644 --- a/vektra-index/src/vektra_index/reindex.py +++ b/vektra-index/src/vektra_index/reindex.py @@ -31,6 +31,8 @@ from vektra_shared.errors import ( ERR_INDEX_001, ERR_INDEX_002, + ERR_INDEX_003, + ERR_INDEX_004, ActiveIndexVersionError, ErrorCategory, ErrorResponse, @@ -119,6 +121,44 @@ def _invalid_index_version(index_version: int) -> HTTPException: return HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) +def _reindex_target_conflict(target_version: int, active_version: int) -> HTTPException: + """400 for a reindex whose target is the version already being served. + + Reindexing a version onto itself would overwrite the live chunks in place + instead of writing a second copy beside them, defeating the zero-downtime + switch. A client can branch on this code to bump the target and retry. + """ + err = ErrorResponse( + category=ErrorCategory.PERMANENT, + code=ERR_INDEX_003, + message=( + f"target_index_version {target_version} must differ from the active " + f"index version {active_version}." + ), + remediation=( + "Pass a target_index_version different from the active version " + "(VEKTRA_ACTIVE_INDEX_VERSION)." + ), + details={ + "target_index_version": target_version, + "active_index_version": active_version, + }, + ) + return HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) + + +def _reindex_job_not_found(job_id: UUID) -> HTTPException: + """404 for a reindex job id that no key-visible job matches.""" + err = ErrorResponse( + category=ErrorCategory.PERMANENT, + code=ERR_INDEX_004, + message=f"Reindex job '{job_id}' not found.", + remediation="Verify the job_id returned by POST /api/v1/reindex.", + details={"job_id": str(job_id)}, + ) + return HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) + + # --------------------------------------------------------------------------- # Background job # --------------------------------------------------------------------------- @@ -380,10 +420,7 @@ async def trigger_reindex( source_version = vs_config.active_index_version if body.target_index_version == source_version: - raise HTTPException( - status_code=400, - detail="target_index_version must differ from the active index version", - ) + raise _reindex_target_conflict(body.target_index_version, source_version) job_id = uuid4() job = ReindexJobOrm( @@ -431,7 +468,7 @@ async def reindex_status( result = await session.execute(select(ReindexJobOrm).where(*filters)) job = result.scalar_one_or_none() if job is None: - raise HTTPException(status_code=404, detail="Reindex job not found") + raise _reindex_job_not_found(job_id) return ReindexStatusResponse( job_id=str(job.id), diff --git a/vektra-index/tests/test_reindex.py b/vektra-index/tests/test_reindex.py index bb9c6cc4..a4640da4 100644 --- a/vektra-index/tests/test_reindex.py +++ b/vektra-index/tests/test_reindex.py @@ -282,3 +282,24 @@ def make_session_ctx(): ) assert call_count == 2 + + +class TestReindexErrorEnvelope: + """DEBT-033: the reindex endpoints now raise the REQ-010 envelope.""" + + def test_target_conflict_is_400_with_index_003(self): + from vektra_index.reindex import _reindex_target_conflict + + exc = _reindex_target_conflict(2, 2) + assert exc.status_code == 400 + assert exc.detail["error"]["code"] == "ERR-INDEX-003" + assert exc.detail["error"]["details"]["active_index_version"] == 2 + + def test_job_not_found_is_404_with_index_004(self): + from vektra_index.reindex import _reindex_job_not_found + + job_id = uuid4() + exc = _reindex_job_not_found(job_id) + assert exc.status_code == 404 + assert exc.detail["error"]["code"] == "ERR-INDEX-004" + assert exc.detail["error"]["details"]["job_id"] == str(job_id) diff --git a/vektra-shared/src/vektra_shared/errors.py b/vektra-shared/src/vektra_shared/errors.py index a1fc649a..52dc08f8 100644 --- a/vektra-shared/src/vektra_shared/errors.py +++ b/vektra-shared/src/vektra_shared/errors.py @@ -109,6 +109,7 @@ def __init__(self, namespace: str, index_version: int) -> None: ERR_QUERY_002 = "ERR-QUERY-002" # LLM unavailable (both primary and fallback failed) ERR_QUERY_003 = "ERR-QUERY-003" # Query too long (exceeds token limit) ERR_QUERY_004 = "ERR-QUERY-004" # Vector store read failed +ERR_QUERY_005 = "ERR-QUERY-005" # Query pipeline provider not registered/available # Configuration errors ERR_CONFIG_001 = "ERR-CONFIG-001" # Missing required configuration variable @@ -129,6 +130,12 @@ def __init__(self, namespace: str, index_version: int) -> None: ERR_ANALYTICS_001 = "ERR-ANALYTICS-001" # Analytics service not available ERR_ANALYTICS_002 = "ERR-ANALYTICS-002" # Trace not found +# Conversation errors (core + admin conversation surface, distinct from the +# learn vertical's ERR-LEARN-005/006 which are scoped to the widget's own path) +ERR_CONV_001 = "ERR-CONV-001" # Conversation not found +ERR_CONV_002 = "ERR-CONV-002" # Persistent conversation store not available +ERR_CONV_003 = "ERR-CONV-003" # Configured store cannot return decrypted turns + # Learn errors (e-learning vertical) ERR_LEARN_001 = "ERR-LEARN-001" # Learn service not available ERR_LEARN_002 = "ERR-LEARN-002" # Enrollment not found @@ -140,6 +147,8 @@ def __init__(self, namespace: str, index_version: int) -> None: # Index errors (ARCH-045, index versioning) ERR_INDEX_001 = "ERR-INDEX-001" # Refused: that index version is the one being served ERR_INDEX_002 = "ERR-INDEX-002" # Invalid index version (< 1) +ERR_INDEX_003 = "ERR-INDEX-003" # Reindex target equals the active version +ERR_INDEX_004 = "ERR-INDEX-004" # Reindex job not found # --------------------------------------------------------------------------- @@ -167,8 +176,12 @@ def __init__(self, namespace: str, index_version: int) -> None: ERR_LEARN_004: 409, ERR_LEARN_005: 404, ERR_LEARN_006: 403, + ERR_CONV_001: 404, + ERR_CONV_003: 501, # Not Implemented: this store cannot decrypt turns ERR_INDEX_001: 409, # Conflict: the version is live, deleting it is refused ERR_INDEX_002: 400, + ERR_INDEX_003: 400, # Bad Request: target must differ from the active version + ERR_INDEX_004: 404, } @@ -210,3 +223,65 @@ def auth_insufficient_scope( ), request_id=request_id or uuid4(), ) + + +def provider_registry_unavailable(request_id: UUID | None = None) -> ErrorResponse: + """500 for a request that reached a handler before the app finished wiring. + + Reuses ERR-CONFIG-001, the same code the global unhandled-exception handler + assigns to internal failures: from a client's point of view a missing + provider registry is an internal setup fault, not something it can fix. + """ + return ErrorResponse( + category=ErrorCategory.CONFIGURATION, + code=ERR_CONFIG_001, + message="The provider registry is not initialized.", + remediation=( + "This indicates the service did not start up correctly. Check the " + "server logs and restart the service." + ), + request_id=request_id or uuid4(), + ) + + +def conversation_not_found( + conversation_id: object, request_id: UUID | None = None +) -> ErrorResponse: + return ErrorResponse( + category=ErrorCategory.PERMANENT, + code=ERR_CONV_001, + message=f"Conversation '{conversation_id}' not found.", + remediation="Verify the conversation id; it may have been deleted.", + request_id=request_id or uuid4(), + ) + + +def conversation_store_unavailable(request_id: UUID | None = None) -> ErrorResponse: + return ErrorResponse( + category=ErrorCategory.TRANSIENT, + code=ERR_CONV_002, + message="Persistent conversation storage is not available.", + remediation=( + "The service may be starting up, or no persistent conversation store " + "is configured (set VEKTRA_CONVERSATION_KEY). Retry shortly." + ), + request_id=request_id or uuid4(), + ) + + +def conversation_turns_unsupported(request_id: UUID | None = None) -> ErrorResponse: + """501 when the configured store cannot return decrypted turns. + + The in-memory store used in tests/dev has no decryption path; the request is + well-formed but this deployment cannot serve it. + """ + return ErrorResponse( + category=ErrorCategory.CONFIGURATION, + code=ERR_CONV_003, + message="The configured conversation store cannot return decrypted turns.", + remediation=( + "Configure a persistent conversation store with VEKTRA_CONVERSATION_KEY " + "set; the in-memory store cannot decrypt turn history." + ), + request_id=request_id or uuid4(), + ) diff --git a/vektra-shared/tests/test_errors.py b/vektra-shared/tests/test_errors.py index 123d729c..7f849522 100644 --- a/vektra-shared/tests/test_errors.py +++ b/vektra-shared/tests/test_errors.py @@ -8,6 +8,11 @@ ERR_AUTH_003, ERR_CONFIG_001, ERR_CONFIG_002, + ERR_CONV_001, + ERR_CONV_002, + ERR_CONV_003, + ERR_INDEX_003, + ERR_INDEX_004, ERR_INGEST_001, ERR_INGEST_002, ERR_INGEST_003, @@ -16,11 +21,16 @@ ERR_QUERY_002, ERR_QUERY_003, ERR_QUERY_004, + ERR_QUERY_005, ErrorCategory, ErrorResponse, auth_insufficient_scope, auth_invalid_token, + conversation_not_found, + conversation_store_unavailable, + conversation_turns_unsupported, http_status_for, + provider_registry_unavailable, ) @@ -200,3 +210,72 @@ def test_auth_insufficient_scope_mentions_scope(self): assert err.code == ERR_AUTH_003 assert "admin" in err.message assert err.remediation != "" + + +class TestDebt033Codes: + """DEBT-033: the pre-existing bare-detail endpoints now use the envelope. + + Each code preserves the HTTP status the endpoint returned before the fix, so + the change is shape-only, not a behaviour change. + """ + + def test_new_codes_have_expected_prefixes(self): + assert ERR_CONV_001.startswith("ERR-CONV-") + assert ERR_CONV_002.startswith("ERR-CONV-") + assert ERR_CONV_003.startswith("ERR-CONV-") + assert ERR_QUERY_005 == "ERR-QUERY-005" + assert ERR_INDEX_003 == "ERR-INDEX-003" + assert ERR_INDEX_004 == "ERR-INDEX-004" + + def test_conversation_not_found_is_404(self): + err = conversation_not_found("abc") + assert err.code == ERR_CONV_001 + assert http_status_for(err) == 404 + assert "abc" in err.message + assert err.remediation != "" + + def test_conversation_store_unavailable_is_503(self): + err = conversation_store_unavailable() + assert err.code == ERR_CONV_002 + assert err.category == ErrorCategory.TRANSIENT + assert http_status_for(err) == 503 + assert err.remediation != "" + + def test_conversation_turns_unsupported_is_501(self): + err = conversation_turns_unsupported() + assert err.code == ERR_CONV_003 + assert http_status_for(err) == 501 + assert err.remediation != "" + + def test_query_pipeline_unavailable_is_503(self): + err = ErrorResponse( + category=ErrorCategory.TRANSIENT, + code=ERR_QUERY_005, + message="x", + remediation="y", + ) + assert http_status_for(err) == 503 + + def test_index_003_maps_to_400(self): + err = ErrorResponse( + category=ErrorCategory.PERMANENT, + code=ERR_INDEX_003, + message="x", + remediation="y", + ) + assert http_status_for(err) == 400 + + def test_index_004_maps_to_404(self): + err = ErrorResponse( + category=ErrorCategory.PERMANENT, + code=ERR_INDEX_004, + message="x", + remediation="y", + ) + assert http_status_for(err) == 404 + + def test_provider_registry_unavailable_reuses_config_001(self): + err = provider_registry_unavailable() + assert err.code == ERR_CONFIG_001 + assert http_status_for(err) == 500 + assert err.remediation != "" From 5343be82a63d99b7db55e1794e3a29ec45dd8ac1 Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Wed, 15 Jul 2026 08:00:28 +0000 Subject: [PATCH 2/4] docs(changelog): record the DEBT-033 error-envelope fix Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 134f3f0c..6a554c4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ Convention (Keep a Changelog 1.1.0): ### Fixed +- **api**: the REQ-010 error envelope (`{"error": {category, code, message, remediation, request_id}}`) is now returned by every API error path, not most of them (DEBT-033). `docs/reference/api.md` first presented it as universal, then (DOCS-009) walked that back with a caveat naming four endpoints that raised a bare FastAPI `{"detail": "..."}` instead — no code to branch on, no remediation. The caveat itself undercounted: grepping the handlers turned up the same gap on `POST /api/v1/query` (registry / pipeline-not-configured), the admin authentication dependency and the API-key creation path (registry / key-store-not-configured), and `GET /api/v1/admin/health?detail=full` (service-initializing) — none of which it listed, so removing the caveat as written would have made the doc claim a universality four more endpoints still broke. All of them now raise `ErrorResponse.to_envelope()` with a machine-readable code, at the same HTTP status they returned before (the change is shape-only, and no test asserted the bare shape): `ERR-CONFIG-001` is reused for an uninitialised provider registry and `ERR-CONFIG-002` for a missing key store — the codes the global unhandled-exception handler already assigns to internal faults — and `ERR-CONV-001/002/003` (conversation not found / store unavailable / store cannot decrypt turns), `ERR-QUERY-005` (query pipeline unavailable), `ERR-ADMIN-008` (deep-health auth not ready) and `ERR-INDEX-003/004` (reindex target equals the active version / reindex job not found) are new, each registered in `vektra_shared/errors.py`, `_CODE_STATUS_OVERRIDE` and `docs/reference/error-codes.md`. The api.md caveat is removed. One representation gap is left standing on purpose: the wire shape is `{"detail": {"error": {...}}}`, because FastAPI wraps an `HTTPException` detail under `detail`, so the envelope sits one level down for every enveloped endpoint (auth included), while the top-level `{"error": {...}}` only ever comes from the global 500 handler — the api.md example shows the logical envelope, and reconciling that is a separate follow-up. - **config**: `VEKTRA_LLM_PROVIDER=""` was accepted and the stack **booted and served with an empty LLM provider**, deferring the misconfiguration to the first query — which is the one thing the ARCH-057 startup sequence exists to prevent (REQ-011, NFR-009). The field is required, but typed as a bare `str`, and `""` is a perfectly valid `str`, so pydantic never raised and step 1 never fired. It is now `min_length=1`, so an empty value fails at startup with the same structured, remediable error an absent one already produced. Found by running `tests/test_startup.py` for the first time (DEBT-031): the test had asserted this exact behaviour since it was written, believing `docker run -e VAR=` *unset* the variable when it in fact sets it to the empty string — so it had been asserting against a value the product happily accepted. Nobody found out, because nothing ever ran it. This is the third time a suite in this repo turned out to be broken the moment someone executed it. The same test also caught **BUG-025** (filed, not fixed here): the structured error is emitted and then `raise SystemExit(1)` travels out of the ASGI lifespan into uvicorn, which logs a `Traceback` after it — so a typo'd env var yields the good message *and* a stack dump, where NFR-009 asks for the former instead of the latter. Fixing that means validating before `uvicorn.run()` rather than inside the lifespan, i.e. touching the boot path BUG-024 broke, so it is tracked on its own; the assertion is `xfail(strict=True)`, which turns the suite red the moment the fix lands and the marker is left behind. - **startup**: a failed ARCH-057 startup step printed a raw Python `Traceback` next to the structured `[STARTUP ERROR]`, where NFR-009 asks for the remediable message *instead of* the stack dump (BUG-025, REQ-011). The 11-step sequence ran inside the ASGI lifespan, so an aborting step's `raise SystemExit(1)` travelled out into uvicorn, which logged the exception before "Application startup failed." — and it applied to every step, not just config validation. The sequence now lives in a loop-agnostic `_run_startup()` that raises `StartupValidationError` (never `SystemExit`), and the container serves through a new `main()` entrypoint (`python -m vektra_app.main`, wired in `docker/entrypoint.sh`) that runs the validation in the very event loop that will serve, then starts uvicorn with `lifespan="off"`. A failed step logs its `[STARTUP ERROR]` and exits non-zero with no ASGI and no traceback; sharing one loop keeps the async resources built during validation (DB engine, HTTP clients) bound to the loop that serves, and `config.get_loop_factory()` preserves uvloop (the CLI default that uvicorn 0.36 stopped exposing through `setup_event_loop()`). The lifespan keeps validating and raising `SystemExit` for the in-process/TestClient path, so `test_missing_llm_provider_aborts_startup` is untouched; the `xfail(strict=True)` on `test_no_raw_traceback_on_startup_failure` is removed. Proven on the container for a config-validation failure and a database_connectivity failure (structured error, exit 1, no traceback) and a valid boot (all 11 steps logged, `startup_complete`, `Uvicorn running`). The required check names (`CI gate`, `Integration tests + NFR gates`) are unchanged. - **tests**: five suites that no runner executed now run, all five found by the guard above the moment it was pointed at `develop` (DEBT-031). Four are `integration`-marked — `vektra-admin/tests/test_integration.py`, `vektra-index/tests/test_integration.py`, `vektra-index/tests/test_benchmark.py`, `vektra-ingest/tests/test_integration.py` — so every unit run excluded them by `-m "not integration"`, while the only job that ran `-m integration` named `vektra-app/tests/` alone: DEBT-030 wired up vektra-app by hand and left the identical hole open in three neighbouring packages. The fifth is `tests/test_startup.py`, the ARCH-057 startup-validation suite, whose own docstring states it requires the stack "started by integration.yml" — it did not run there, because that workflow names `tests/integration/` and nothing named `tests/`. It was watching the precise surface BUG-024 broke, and it was watching nothing. The `app-integration` job becomes `package-integration` and targets `vektra-*/tests -m integration`, a glob on purpose: the hand-written package list is the thing that has now failed twice, and a package added tomorrow is picked up with no edit. `tests/test_startup.py` joins the job that already brings the stack up. Unlike vektra-app's suites in DEBT-030 these four had **not** rotted — 52 integration tests pass — but that was luck, not a property of the arrangement. The five suites under `tests/` were also missing the `integration` marker they require, which is what made them indistinguishable from unit tests `make test` had merely forgotten; `tests/nfr/test_performance.py` remains deliberately unrun (it needs a live LLM that GitHub Actions does not have) and is the single justified entry in the guard's `EXPECTED_UNRUN`, where a reviewer can see it. The required check names (`CI gate`, `Integration tests + NFR gates`) are unchanged. From 229b622586b1a88e0819e6d43735ebc83a718a49 Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Wed, 15 Jul 2026 08:02:38 +0000 Subject: [PATCH 3/4] docs(backlog): mark DEBT-033 done and file DEBT-034 (#112) Co-Authored-By: Claude Opus 4.8 (1M context) --- .s2s/BACKLOG.md | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/.s2s/BACKLOG.md b/.s2s/BACKLOG.md index 3da37fb8..01412458 100644 --- a/.s2s/BACKLOG.md +++ b/.s2s/BACKLOG.md @@ -974,7 +974,7 @@ Consequences: every reindex permanently doubles the storage for that namespace, ### DEBT-033: some endpoints bypass the REQ-010 error envelope -**Status**: planned | **Priority**: low | **Created**: 2026-07-14 +**Status**: completed | **Priority**: low | **Created**: 2026-07-14 | **Completed**: 2026-07-15 | **PR**: #112 **Origin**: DOCS-009 (2026-07-14). **Context**: REQ-010 defines a single error envelope (`{"error": {category, code, message, remediation, request_id, details}}`), and `docs/reference/api.md` presented it as universal. It is not. Reindex (`400`, `404`), conversations (`404`, `503`) and admin conversation turns (`404`, `501`, `503`) raise `HTTPException` with a plain string detail, so they return FastAPI's bare `{"detail": "..."}` — no code, no remediation, no request id. @@ -984,8 +984,28 @@ Consequences: every reindex permanently doubles the storage for that namespace, A client cannot branch on an error code for these endpoints, and the operator-facing reindex failures are exactly the ones where a machine-readable code would be worth having. The doc now warns that both shapes exist; the fix is to make the envelope actually universal. **Acceptance criteria**: -- [ ] The endpoints above raise envelope errors with proper `ERR-*` codes -- [ ] The "not every endpoint uses the envelope" caveat is removed from `docs/reference/api.md` +- [x] The endpoints above raise envelope errors with proper `ERR-*` codes +- [x] The "not every endpoint uses the envelope" caveat is removed from `docs/reference/api.md` + +**Resolution** (2026-07-15): the four scoped endpoints were enveloped, and a grep of the handlers (`HTTPException(..., detail="...")`) found the DOCS-009 caveat had **undercounted** — the same bare shape lived on `POST /api/v1/query` (registry / pipeline-not-configured), the admin auth dependency and the API-key creation path (registry / key-store-not-configured), and `GET /api/v1/admin/health?detail=full` (service-initializing). Removing the caveat as written would have re-asserted a universality those still broke, so all of them were enveloped too (approved scope expansion). Each raise keeps its prior HTTP status (shape-only change; no test asserted the bare shape). Codes: reused `ERR-CONFIG-001` (registry not initialized) and `ERR-CONFIG-002` (key store not configured), matching the global handler's treatment of internal faults; added `ERR-CONV-001/002/003`, `ERR-QUERY-005`, `ERR-ADMIN-008`, `ERR-INDEX-003/004`, each registered in `errors.py` + `_CODE_STATUS_OVERRIDE` + `docs/reference/error-codes.md`. The api.md caveat is removed. Spawned **DEBT-034**: the wire shape is `{"detail": {"error": {...}}}` for every `HTTPException`-based error (FastAPI wraps the detail), while api.md shows the logical top-level `{"error": {...}}`; that representation gap predates this work and affects every enveloped endpoint. + +--- + +### DEBT-034: the error envelope is nested under `detail` on the wire, but documented as top-level + +**Status**: planned | **Priority**: low | **Created**: 2026-07-15 +**Origin**: DEBT-033 (2026-07-15). + +**Context**: FastAPI serializes `raise HTTPException(status_code=..., detail=err.to_envelope())` as `{"detail": {"error": {...}}}` — the REQ-010 envelope sits one level down, under `detail`. This is how **every** enveloped endpoint behaves (auth, ingest, index, analytics, learn, admin, and the endpoints fixed in DEBT-033), and the whole test suite already asserts `resp.json()["detail"]["error"]["code"]`, so the nesting is the de-facto contract. The one exception is the global unhandled-exception handler in `vektra-app/main.py`, which returns top-level `{"error": {...}}` via `JSONResponse`. So there are two shapes after all — not "envelope vs bare" (DEBT-033 closed that), but "envelope under `detail`" (explicit raises) vs "envelope at top level" (uncaught 500s). `docs/reference/api.md` and `docs/reference/error-codes.md` show only the top-level form, so a client that follows the docs looks for `body.error.code` and finds nothing; the real path for almost every error is `body.detail.error.code`. + +**Options**: +1. **(Recommended)** Register a custom `StarletteHTTPException` handler that, when `exc.detail` is already an envelope dict, returns it at top level via `JSONResponse(status_code=exc.status_code, content=exc.detail)`. One handler, no per-endpoint change, and both error paths converge on the documented top-level `{"error": {...}}`. Rationale: makes the wire match the docs and REQ-010's own shape, rather than teaching clients an accidental FastAPI wrapping. +2. Document the nesting instead: change api.md/error-codes.md to show `{"detail": {"error": {...}}}` for `HTTPException`-based errors. Cheaper, but enshrines the artifact and leaves the two-shape split (nested vs top-level from the 500 handler) permanently in the contract. +3. Leave as-is. Rejected: the docs actively mislead about where the code lives. + +**Acceptance criteria**: +- [ ] The REQ-010 envelope is reachable at a single, documented JSON path across all error responses (explicit raises and uncaught 500s alike) +- [ ] `docs/reference/api.md` and `docs/reference/error-codes.md` match the actual wire shape --- From 35b521cc4bf574d37ce15763f99904a353a60692 Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Wed, 15 Jul 2026 08:31:54 +0000 Subject: [PATCH 4/4] fix(api): raise the new errors via shared factories (review) Address the review: promote ERR-ADMIN-008 to a constant and add service_initializing(), query_pipeline_unavailable() and key_store_unavailable() factories in vektra_shared.errors, then raise through them instead of constructing the envelope inline (matching the auth_* / conversation_* factory pattern already in that module). This also removes the duplicated ERR-ADMIN-008 block in the health handler. No status changes; the TRANSIENT codes still resolve to 503 via the category default, so no _CODE_STATUS_OVERRIDE entries are needed. Co-Authored-By: Claude Opus 4.8 (1M context) --- vektra-admin/src/vektra_admin/api.py | 31 +++++----------- vektra-core/src/vektra_core/api.py | 12 ++----- vektra-shared/src/vektra_shared/errors.py | 43 +++++++++++++++++++++++ vektra-shared/tests/test_errors.py | 26 ++++++++++---- 4 files changed, 74 insertions(+), 38 deletions(-) diff --git a/vektra-admin/src/vektra_admin/api.py b/vektra-admin/src/vektra_admin/api.py index b354b673..24adf88a 100644 --- a/vektra-admin/src/vektra_admin/api.py +++ b/vektra-admin/src/vektra_admin/api.py @@ -38,7 +38,6 @@ from vektra_shared.auth import ApiKeyInfo, KeyStoreProvider, require_scope from vektra_shared.db import get_session from vektra_shared.errors import ( - ERR_CONFIG_002, ErrorCategory, ErrorResponse, auth_invalid_token, @@ -46,7 +45,9 @@ conversation_store_unavailable, conversation_turns_unsupported, http_status_for, + key_store_unavailable, provider_registry_unavailable, + service_initializing, ) # --------------------------------------------------------------------------- @@ -122,15 +123,7 @@ async def _require_any_token( try: key_store: KeyStoreProvider = registry.get("key_store", "default") except ValueError: - err = ErrorResponse( - category=ErrorCategory.CONFIGURATION, - code=ERR_CONFIG_002, - message="The API key store is not configured.", - remediation=( - "Verify the key store provider is registered. Check the server " - "logs and restart the service." - ), - ) + err = key_store_unavailable() raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) info = await key_store.lookup_by_token(token) @@ -214,13 +207,10 @@ async def health( ) # Validate token; fail closed if registry/key_store unavailable if registry is None: - err = ErrorResponse( - category=ErrorCategory.TRANSIENT, - code="ERR-ADMIN-008", - message="The service is still initializing.", - remediation="Retry shortly; the service is starting up.", + err = service_initializing() + raise HTTPException( + status_code=http_status_for(err), detail=err.to_envelope() ) - raise HTTPException(status_code=503, detail=err.to_envelope()) try: key_store = registry.get("key_store", "default") info = await key_store.lookup_by_token(credentials.credentials) @@ -232,13 +222,10 @@ async def health( # Expose key_id for AuditMiddleware (CR55) request.state.key_id = info.key_id except ValueError: - err = ErrorResponse( - category=ErrorCategory.TRANSIENT, - code="ERR-ADMIN-008", - message="The service is still initializing.", - remediation="Retry shortly; the service is starting up.", + err = service_initializing() + raise HTTPException( + status_code=http_status_for(err), detail=err.to_envelope() ) - raise HTTPException(status_code=503, detail=err.to_envelope()) status_code = 503 if deep.status == "unhealthy" else 200 return Response( diff --git a/vektra-core/src/vektra_core/api.py b/vektra-core/src/vektra_core/api.py index 05d90764..5faffc92 100644 --- a/vektra-core/src/vektra_core/api.py +++ b/vektra-core/src/vektra_core/api.py @@ -34,13 +34,13 @@ from vektra_shared.errors import ( ERR_QUERY_002, ERR_QUERY_003, - ERR_QUERY_005, ErrorCategory, ErrorResponse, conversation_not_found, conversation_store_unavailable, http_status_for, provider_registry_unavailable, + query_pipeline_unavailable, ) from vektra_shared.namespace import resolve_citations_enabled, resolve_grounding_mode from vektra_shared.types import ( @@ -214,15 +214,7 @@ async def query( try: pipeline = registry.get("query_pipeline", "default") except ValueError: - err = ErrorResponse( - category=ErrorCategory.TRANSIENT, - code=ERR_QUERY_005, - message="The query pipeline is not available.", - remediation=( - "The service may be starting up, or no query pipeline is " - "configured. Retry shortly." - ), - ) + err = query_pipeline_unavailable() raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) # Safeguard pre_query (input validation trust boundary, REQ-044) diff --git a/vektra-shared/src/vektra_shared/errors.py b/vektra-shared/src/vektra_shared/errors.py index 52dc08f8..29f4c52a 100644 --- a/vektra-shared/src/vektra_shared/errors.py +++ b/vektra-shared/src/vektra_shared/errors.py @@ -150,6 +150,10 @@ def __init__(self, namespace: str, index_version: int) -> None: ERR_INDEX_003 = "ERR-INDEX-003" # Reindex target equals the active version ERR_INDEX_004 = "ERR-INDEX-004" # Reindex job not found +# Admin errors (ERR-ADMIN-001..007 live in vektra-admin as string literals; +# 008 is shared because it backs a factory reused within that module) +ERR_ADMIN_008 = "ERR-ADMIN-008" # Deep-health auth: registry/key store not yet ready + # --------------------------------------------------------------------------- # HTTP status mapping helpers @@ -285,3 +289,42 @@ def conversation_turns_unsupported(request_id: UUID | None = None) -> ErrorRespo ), request_id=request_id or uuid4(), ) + + +def query_pipeline_unavailable(request_id: UUID | None = None) -> ErrorResponse: + """503 when the query pipeline provider is not registered yet.""" + return ErrorResponse( + category=ErrorCategory.TRANSIENT, + code=ERR_QUERY_005, + message="The query pipeline is not available.", + remediation=( + "The service may be starting up, or no query pipeline is configured. " + "Retry shortly." + ), + request_id=request_id or uuid4(), + ) + + +def key_store_unavailable(request_id: UUID | None = None) -> ErrorResponse: + """500 when the API key store provider is not configured.""" + return ErrorResponse( + category=ErrorCategory.CONFIGURATION, + code=ERR_CONFIG_002, + message="The API key store is not configured.", + remediation=( + "Verify the key store provider is registered. Check the server logs " + "and restart the service." + ), + request_id=request_id or uuid4(), + ) + + +def service_initializing(request_id: UUID | None = None) -> ErrorResponse: + """503 for a deep-health auth check that ran before the store was ready.""" + return ErrorResponse( + category=ErrorCategory.TRANSIENT, + code=ERR_ADMIN_008, + message="The service is still initializing.", + remediation="Retry shortly; the service is starting up.", + request_id=request_id or uuid4(), + ) diff --git a/vektra-shared/tests/test_errors.py b/vektra-shared/tests/test_errors.py index 7f849522..942dde53 100644 --- a/vektra-shared/tests/test_errors.py +++ b/vektra-shared/tests/test_errors.py @@ -3,6 +3,7 @@ from uuid import UUID, uuid4 from vektra_shared.errors import ( + ERR_ADMIN_008, ERR_AUTH_001, ERR_AUTH_002, ERR_AUTH_003, @@ -30,7 +31,10 @@ conversation_store_unavailable, conversation_turns_unsupported, http_status_for, + key_store_unavailable, provider_registry_unavailable, + query_pipeline_unavailable, + service_initializing, ) @@ -224,6 +228,7 @@ def test_new_codes_have_expected_prefixes(self): assert ERR_CONV_002.startswith("ERR-CONV-") assert ERR_CONV_003.startswith("ERR-CONV-") assert ERR_QUERY_005 == "ERR-QUERY-005" + assert ERR_ADMIN_008 == "ERR-ADMIN-008" assert ERR_INDEX_003 == "ERR-INDEX-003" assert ERR_INDEX_004 == "ERR-INDEX-004" @@ -248,13 +253,22 @@ def test_conversation_turns_unsupported_is_501(self): assert err.remediation != "" def test_query_pipeline_unavailable_is_503(self): - err = ErrorResponse( - category=ErrorCategory.TRANSIENT, - code=ERR_QUERY_005, - message="x", - remediation="y", - ) + err = query_pipeline_unavailable() + assert err.code == ERR_QUERY_005 + assert http_status_for(err) == 503 + assert err.remediation != "" + + def test_service_initializing_is_503(self): + err = service_initializing() + assert err.code == ERR_ADMIN_008 assert http_status_for(err) == 503 + assert err.remediation != "" + + def test_key_store_unavailable_is_500(self): + err = key_store_unavailable() + assert err.code == ERR_CONFIG_002 + assert http_status_for(err) == 500 + assert err.remediation != "" def test_index_003_maps_to_400(self): err = ErrorResponse(