fix(api): return the REQ-010 error envelope on every error path (DEBT-033)

.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
 
 ---
 

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.

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.

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
 

vektra-admin/src/vektra_admin/api.py

@@ -41,7 +41,13 @@
     ErrorCategory,
     ErrorResponse,
     auth_invalid_token,
+    conversation_not_found,
+    conversation_store_unavailable,
+    conversation_turns_unsupported,
     http_status_for,
+    key_store_unavailable,
+    provider_registry_unavailable,
+    service_initializing,
 )
 
 # ---------------------------------------------------------------------------
@@ -111,12 +117,14 @@ 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 = key_store_unavailable()
+        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 +207,10 @@ 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 = service_initializing()
+            raise HTTPException(
+                status_code=http_status_for(err), detail=err.to_envelope()
+            )
         try:
             key_store = registry.get("key_store", "default")
             info = await key_store.lookup_by_token(credentials.credentials)
@@ -211,7 +222,10 @@ 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 = service_initializing()
+            raise HTTPException(
+                status_code=http_status_for(err), detail=err.to_envelope()
+            )
 
         status_code = 503 if deep.status == "unhealthy" else 200
         return Response(
@@ -290,8 +304,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 +531,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)