152 lines
8.3 KiB
Markdown
152 lines
8.3 KiB
Markdown
---
|
||
id: review-001-openapi-projection-fidelity
|
||
name: to_openapi golden fidelity — runtime-truthful document (PRJ-01..PRJ-05, PRJ-11, PRJ-12, PRJ-14, PRJ-15)
|
||
status: completed
|
||
depends_on: [review-001-gateway-stream-errors, review-001-gateway-publish-semantics]
|
||
scope: moderate
|
||
risk: medium
|
||
impact: component
|
||
level: implementation
|
||
tags: [adapters, review-001, to-openapi]
|
||
---
|
||
|
||
## Description
|
||
|
||
Review 001 Part F: the `to_openapi` document describes responses the
|
||
gateway does not emit. The runtime contract must be settled **before**
|
||
this doc work — hence the gateway dependencies (per the review's
|
||
sequencing note). Systematic diff of the projection against the runtime
|
||
(routes tests are the oracle):
|
||
|
||
- **PRJ-01/PRJ-02**: `/search` and `/schema` 200 responses are documented
|
||
without the envelope wrapper (`{request_id, result, output}`) and with
|
||
wrong item fields (`description` that doesn't exist; missing
|
||
`namespace`/`op_type`/`visibility`/`access_control`/`channel_open`/
|
||
`publish_schema`). Clients generated from the doc are broken day one.
|
||
- **PRJ-03**: documented 400 `INVALID_INPUT` vs runtime 422
|
||
(`error.rs:51`); axum extractor rejections are plain-text bodies with
|
||
none of the documented `{code, message, retryable}` shape; no 422 in
|
||
the doc at all.
|
||
- **PRJ-04**: op errors declared at non-`HTTP_*` codes are projected
|
||
under statuses the runtime never produces (`ErrorDefinition.http_status`
|
||
is never consulted; `RATE_LIMITED` surfaces as 500). Either project
|
||
honestly under 500 or honor `http_status` at runtime — with the review's
|
||
note that `operation_errors_projected_onto_call` currently enshrines the
|
||
wrong behavior.
|
||
- **PRJ-05**: `/subscribe` documented statuses are structurally
|
||
unreachable (always 200 + SSE per GW-12); in-band `event:error` frames
|
||
undocumented. Document the 200+in-band-error contract.
|
||
- **PRJ-11/HY-03**: remaining guarded `expect`s in `to_openapi`
|
||
(`:440, :505, :512`) — replace with `if let` (convention).
|
||
- **PRJ-12**: error dedupe iterates a `HashMap` → nondeterministic doc
|
||
regeneration for identical registry state. Dedupe by `(code, status)`
|
||
or sort.
|
||
- **PRJ-14**: `schema_call_request()` inlined 4× and unused-in-practice
|
||
`components.schemas` — use `$ref` or drop components so shape changes
|
||
can't drift.
|
||
- **PRJ-15**: `/search` documents 401/403 that cannot occur, omits the
|
||
404 that can; no `securitySchemes` anywhere despite Bearer being the
|
||
contract (ADR-004).
|
||
|
||
## Acceptance Criteria
|
||
|
||
- [x] Generated-doc golden test asserted against hand-written expectations matching the routes tests' actual bodies (the review's gate for this unit)
|
||
- [x] `/subscribe` doc documents the 200 + `event:error` in-band contract (GW-12's asymmetry called out); `/search` statuses match reality
|
||
- [x] PRJ-04 decision landed (project-honest vs honor-`http_status`) and the enshrining test aligned
|
||
- [x] Deterministic doc generation (same registry → byte-identical doc, test)
|
||
- [x] `$ref`s in components or components removed; `expect`s replaced
|
||
- [x] `components.securitySchemes` declares the Bearer scheme
|
||
- [x] `cargo test` and `cargo clippy --all-targets -- -D warnings` pass
|
||
|
||
## References
|
||
|
||
- docs/reviews/001-initial-implementation-review.md (Part F, PRJ-01..PRJ-05, PRJ-11, PRJ-12, PRJ-14, PRJ-15)
|
||
- docs/architecture/decisions/042-openapi-gateway-pattern.md, 045-to-openapi-gateway-spec-versioning.md
|
||
- docs/architecture/decisions/023-operation-error-schemas.md (error fidelity)
|
||
- src/gateway/routes.rs tests (the runtime oracle), src/gateway/error.rs (the mapping)
|
||
|
||
## Notes
|
||
|
||
> Sequenced after the gateway tasks; the documented contract now
|
||
> matches the settled runtime (review's Unit 6 ordering). The two
|
||
> decisions that shaped the implementation:
|
||
>
|
||
> - **PRJ-03 (doc-align, no runtime change)**: the doc maps dispatch-path
|
||
> `INVALID_INPUT`/`INVALID_OPERATION_TYPE` to `422` (the shared
|
||
> mapper's real output) and documents the axum extractor-rejection gap
|
||
> explicitly on the `400` responses (`text/plain` body, marked as a
|
||
> known runtime gap). Closing the extractor gap is a runtime change on
|
||
> routes.rs — another surface — so the doc states what the runtime
|
||
> actually does.
|
||
> - **PRJ-04 (project-honest, no runtime change)**: registry-declared
|
||
> `http_status` is still projected in the doc (so the declared codes
|
||
> stay visible), but every projection whose codes are not
|
||
> `HTTP_<status>`-prefixed carries `x-runtime-behavior: 500` plus an
|
||
> explicit description note — the runtime mapper is purely code-driven
|
||
> and `ErrorDefinition.http_status` is never consulted. The enshrining
|
||
> test was rewritten to assert the annotation instead of the old
|
||
> behavior. Honoring `http_status` at runtime would change
|
||
> routes.rs/error.rs (another agent's surface) and the ADR-023
|
||
> semantics — rejected here.
|
||
|
||
## Summary
|
||
|
||
**What landed** (`src/adapters/to_openapi.rs` overhaul, plus a one-line
|
||
version assertion in `src/server/adapter.rs` tests):
|
||
|
||
- **PRJ-01** — `/search` 200 now documents the envelope
|
||
(`{request_id, result, output}` with the `operations` array under
|
||
`output`) and the real item fields (`name`, `namespace`, `op_type`;
|
||
no `description`). The `description`-field lie is gone from both the
|
||
schema and the summary.
|
||
- **PRJ-02** — `/schema` 200 documents the envelope wrapping a full
|
||
`OperationSpecOutput` component (`op_type`, `visibility`,
|
||
`input_schema`, `output_schema`, `error_schemas`, `access_control`,
|
||
`channel_open`, `publish_schema`).
|
||
- **PRJ-03** — 422 (`CallErrorInvalidInput`/`CallErrorInvalidOperationType`
|
||
oneOf) replaces the bogus 400 INVALID_INPUT on dispatch paths;
|
||
`/call`//`/batch`//`/subscribe`/`/schema` 400s document the
|
||
plain-text extractor-rejection bodies with an explicit gap note;
|
||
`/batch` 400 also covers the 100-op `BatchCapExceeded` JSON shape.
|
||
- **PRJ-04** — decision: **project-honest with annotation**
|
||
(`x-runtime-behavior: 500`); see Notes. Enshrining test replaced by
|
||
`operation_errors_projected_onto_call_with_runtime_behavior_annotated`.
|
||
- **PRJ-05** — `/subscribe` documents exactly `200` (SSE) + `400`
|
||
(extractor). The 200 description spells out the GW-12 asymmetry:
|
||
pre-dispatch denials and handler failures arrive as terminal
|
||
`event:error` frames carrying the serialized `CallError`, with the
|
||
`retry: 15000` hint and 15 s keep-alive comment frames — no HTTP error
|
||
statuses are structurally reachable.
|
||
- **PRJ-11** — the last library-side `expect`s are gone; all emission
|
||
paths use infallible construction (`as_object_mut` guards are gone
|
||
with the deleted mutation-based merge machinery, replaced by
|
||
`struct`-built JSON).
|
||
- **PRJ-12** — `gather_operation_errors` folds registry iteration into
|
||
`BTreeMap`/`BTreeMap<String, _>` before anything is emitted; status
|
||
keys and per-status code enums are sorted. `same_registry_yields_byte_identical_docs`
|
||
pins determinism; `error_codes_are_sorted_within_each_status_key`
|
||
pins the ordering.
|
||
- **PRJ-14** — request bodies (`/call`, `/batch` items, `/subscribe`)
|
||
reference `#/components/schemas/CallRequest`; the doc now defines
|
||
`components.schemas` (19 components) + `components.responses`
|
||
(6 shared error responses) and every endpoint's status map references
|
||
them. No inline duplication of the `CallRequest` shape remains.
|
||
- **PRJ-15** — `/search` drops the unreachable 401/403 and documents the
|
||
real set (200/404/500/504); `components.securitySchemes.bearerAuth`
|
||
(HTTP bearer, ADR-004) is declared and applied via top-level
|
||
`security` + per-operation overrides.
|
||
- **ADR-045** — `info.version` bumped `1.1.0` → **`1.2.0`** (minor):
|
||
the overhaul is additive at the shape level (newly documented
|
||
statuses/envelope fields; nothing previously documented was removed
|
||
or retyped into a break). The `server::adapter` integration test was
|
||
aligned to the new version.
|
||
- **Golden test** — 31 tests in `src/adapters/to_openapi.rs` assert the
|
||
generated doc against hand-written expectations mirroring the routes
|
||
tests' actual bodies (envelope shape, item fields, status sets, code
|
||
enums, security scheme, print-level determinism), and the full doc
|
||
re-validates via `openapiv3`.
|
||
|
||
**Verification:** `cargo test` (288 passed, including the full-surface
|
||
integration tests), `cargo clippy --all-targets -- -D warnings` (clean),
|
||
`cargo fmt --check` (clean) — verified in a clean worktree at HEAD
|
||
because other agents' in-flight edits were present in the shared tree. |