From 54f8e2310ef970bc6cd92eafa9d2610b9c0c473c Mon Sep 17 00:00:00 2001 From: "glm-5.3-flash" Date: Fri, 28 Aug 2026 17:22:46 +0000 Subject: [PATCH] =?UTF-8?q?docs(review=20001):=20initial=20implementation?= =?UTF-8?q?=20review=20=E2=80=94=20findings,=20coverage,=20remediation=20p?= =?UTF-8?q?lan?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidated review of the 17-task initial implementation (tree 4a825d3): 8 subsystem passes + cargo-llvm-cov coverage analysis. Baseline: 256 tests green, clippy/fmt/doc clean, 93.86% line coverage. Findings: 1 borderline-critical ($ref recursion aborts the process), ~30 major (extra_routes mounted without auth middleware, /schema leaks Internal ops, /mcp unbounded body, forwarding URL construction/SSRF, redirect credential leakage, non-idempotent retries, no default timeouts, SSE chunk-boundary event loss, MCP schema ACL skip, from_wss lost-EOF hang, projection-vs-runtime fidelity), and ~45 minor. Includes verified-solid list, coverage gap analysis, and a 7-unit remediation plan. --- .../001-initial-implementation-review.md | 1437 +++++++++++++++++ 1 file changed, 1437 insertions(+) create mode 100644 docs/reviews/001-initial-implementation-review.md diff --git a/docs/reviews/001-initial-implementation-review.md b/docs/reviews/001-initial-implementation-review.md new file mode 100644 index 0000000..d0d714e --- /dev/null +++ b/docs/reviews/001-initial-implementation-review.md @@ -0,0 +1,1437 @@ +# Review 001 — Initial Implementation Review (the 17-task build) + +## Status + +Verified, open for remediation. + +## Scope + +Consolidated code review of the full initial implementation of alkhttp — +the 17-task build that landed the server foundation, the WebSocket +byte-adapter and channels session, the gateway (dispatch + 6 routes + +publish), the reqwest client host, and the five adapters +(`from_openapi`, `from_jsonschema`, `from_wss`, `from_mcp`, `to_openapi`, +`to_mcp`) — produced against the tree at `4a825d3` ("full-surface +integration suite + docs sync + publish prep"). + +The review consolidates eight passes (one per subsystem: server core, +WebSocket, gateway, client host + forwarding core, from_openapi/from_jsonschema, +to_openapi/to_mcp, from_wss/from_mcp, and a cross-cutting hygiene pass) +plus a `cargo-llvm-cov` coverage analysis. The most consequential +findings (auth layering, `/schema` visibility, forwarding URL +construction, SSE parsing, `Notify` semantics) were re-verified directly +in source and, where noted, empirically with scratch harnesses outside +the repo. Findings that are cross-crate (alkcall-side) are flagged as +such. + +## Baseline verification (this pass) + +``` +cargo test --all-features → 256 passed, 0 failed (227 unit + 29 integration) +cargo clippy --all-features --all-targets -- -D warnings → clean +cargo fmt --check → clean +cargo doc --no-deps → 0 warnings +cargo check --no-default-features → clean +cargo llvm-cov --all-features → 93.86% lines, 91.75% functions, 94.23% regions +``` + +The suite is green and the coverage number is high, but the integration +tests all exercise happy paths over real I/O; the gaps are at the edges +(auth layering on merged routers, malformed/hostile upstreams, error +paths, projection-vs-runtime fidelity). That is exactly where the +findings below live. + +## Verdict + +- **The architecture held.** The gateway pattern (fixed endpoints, no + per-op surface), the no-env-vars credential invariant, the + Internal-by-default privilege model, the HTTP_ error-code + discipline, the WS byte-adapter framing, and the feature matrix are all + implemented as specified and largely well tested. There is no + "cannot function end-to-end" defect of the kind alkcall Review 001 + found (P-01/C-01); the happy paths are genuinely exercised by + integration tests over real sockets. +- **The defects cluster in four places:** (1) *auth/visibility at the + seams* — `extra_routes` mounts without the bearer middleware, `/schema` + leaks Internal ops, and the MCP `schema` tool skips the per-op ACL + check; (2) *the outbound forwarding core* — URL construction + (unencoded path params, `Url::join` semantics, no host validation), + cross-host redirect credential leakage, retries on non-idempotent + methods, missing timeouts, and an SSE parser that silently drops events + at TCP chunk boundaries; (3) *projection fidelity* — the `to_openapi` + document describes responses the gateway does not emit; (4) + *WS/adapter robustness edges* — a lossy EOF notification that can hang + `from_wss` calls forever, head-of-line stalls, and uncapped inbound + buffering multipliers. +- **One borderline-critical defect:** `from_openapi` stack-overflows the + process (abort, not a catchable panic) on valid, common, + self-referential OpenAPI specs. + +## Severity legend + +- **[critical]** — the protocol cannot function end-to-end as committed; + or a decided spec invariant is violated in a way that corrupts data or + causes silent permanent state damage. +- **[major]** — a decided behavior is missing, wrong, or a real + reliability hazard; works in the happy path but fails a spec-required + edge case, or is a realistic security weakness. +- **[minor]** — drift, convention violation, dead code, or a doc/spec + inconsistency with no correctness impact. + +--- + +# Part A — Server core (adapter, auth, decoy, state) + +## SRV-01 [major] — `extra_routes` are mounted without the bearer-auth middleware + +**ADR drift:** ADR-046 §4 ("Custom routes carry the same auth middleware +by default; per-route opt-out is the deployment's choice"). +**Verified:** YES (empirically). `src/server/adapter.rs:170-183` — the +`bearer_auth_middleware` is applied via `route_layer` *before* extra +routes are merged. axum's `route_layer` wraps only routes registered +*before* the call (verified against axum 0.8.9 `path_router.rs` and with +a scratch harness replicating `build_router`'s shape: the middleware ran +only for default-surface routes; an extra-route handler saw no stashed +identity). + +Amplifier: `ResolvedIdentity` extraction never fails +(`src/server/auth.rs` — `Rejection = Infallible`), so a custom-route +handler using `ResolvedIdentity` compiles and silently receives `None` +on every request — indistinguishable from "caller sent no token". If a +custom handler treats `None` as anonymous-allowed, that is an +unauthenticated access path; at minimum the documented default (auth +applied) is not the implemented default. **Fix:** merge extras *first* +(or apply the layer after the merge), so the documented default holds and +opt-out remains the deployment's explicit choice. + +## SRV-02 [major] — `/schema` discloses the full spec of `Visibility::Internal` operations + +**ADR drift:** ADR-015 §2 / http-server.md ("Internal operations return +404 … an HTTP client cannot stub its toe on a path for an operation it +can't call"). +**Verified:** YES. `src/gateway/routes.rs:120-137` — `schema_handler` +runs only `access_check_for_op`; it never applies the `is_internal_op` +pre-check that `/call` (:99), `/batch` (:147), `/subscribe` (:164), and +`/publish` (:240) all apply. Internal ops carry `AccessControl::default()` +(no restrictions), so the pre-check passes even for an unauthenticated +caller, and the `services/schema` handler returns the full spec +(visibility, input/output/error schemas, `access_control`) for any +registered name. `POST /call` on the same op correctly 404s (asserted in +`tests/full_surface.rs`). One unauthenticated `GET /schema?name=…` +returns the internal op's contract, defeating the invisibility invariant +on the discovery axis and bypassing the per-caller `/search` surface. +There is no test covering `/schema` on an internal op. + +## SRV-03 [major] — `/mcp` reads the request body with no size limit (unbounded memory) + +**Verified:** YES (mcp feature only). `src/server/adapter.rs:141-150` +nests rmcp's `StreamableHttpService`; the bearer middleware only stashes +identity. axum's 2 MiB `DefaultBodyLimit` applies to axum *extractors*; +the nested rmcp service consumes the raw body via `body.collect().await` +with no cap (verified against rmcp 1.8.0 `server_side_http.rs`), and +hyper imposes no body limit. A single `POST /mcp` with a multi-GB +chunked body is buffered entirely in memory; a handful of concurrent +requests OOMs the process. The gateway routes themselves are correctly +capped at 2 MiB (axum extractors). **Fix:** wrap the nest with an +explicit `DefaultBodyLimit` (or a body-limit layer) sized for MCP. + +## SRV-04 [major] — No read/idle timeouts on the hyper connection driver (slow-loris surface) + +**Verified:** YES. `src/server/adapter.rs:217-224` — +`HyperBuilder::new(TokioExecutor::new())` sets no timer, no +`header_read_timeout`, no keep-alive/h2 keep-alive knobs. hyper's h1 +`header_read_timeout` (30 s default) is **silently ignored without a +Timer** (verified against hyper 1.11 `common/time.rs`: `Time::Empty` → +`warn!` + `None`). A client that opens a connection and dribbles partial +headers parks a task + buffers indefinitely; each `HttpAdapter::handle` +admits one such connection and there is no concurrency cap in this crate +(the accept loop is the consumer's). Mitigation is partly the transport +layer's job, but this crate configures none of the knobs hyper provides. + +## SRV-05 [minor] — `with_decoy` consumes `extra_routes` via `.take()` + +**Verified:** YES. `src/server/adapter.rs:104` — +`build_router(state, self.extra_routes.take())` leaves +`self.extra_routes == None`; a subsequent `.with_decoy(d2)` rebuilds the +router *without* the custom routes, silently. The builder state and the +live router diverge. Should be a clone (or build-once semantics with an +explicit error). + +## SRV-06 [minor] — `RESERVED_PATHS` is exported but never enforced; per-method merges leak through + +**Verified:** YES. `src/server/adapter.rs:38-49` — the constant has no +reader outside its re-export. Actual collision behavior: same-method +overlap **panics** in axum's `merge` (sanctioned by ADR-046 §3's +"construction panics/warns"), but a *different* method on a reserved path +(e.g. custom `POST /search`) silently merges in — served on a reserved +path, and (per SRV-01) without auth middleware. The "default wins" claim +of ADR-046 §3 is false for the per-method case, and the exported +constant doesn't deliver the contract its export implies. + +## SRV-07 [minor] — Stealth-mode fidelity gap: 405 responses bypass the decoy + +**Verified:** YES (empirically). The decoy is only the router +`fallback` (`src/server/decoy.rs:24-43`); a request that matches a route +but not a method (e.g. `OPTIONS /search`, `DELETE /healthz`) returns +axum's bare 405 — no body, **no `Server: nginx` header** — so a single +probe distinguishes alkhttp from nginx in decoy deployments. Real nginx +sends its `Server` header on 405s. + +## SRV-08 [minor] — Decoy static-server path decoding bugs + +**Verified:** YES. `src/server/decoy.rs:106-127` — in a URI path `+` is +a literal plus (only query strings form-decode it), but `percent_decode` +maps it to space (a file named `a+b.html` becomes unreachable); and +percent-decoding maps each decoded byte to `char`, so `%C3%A9` yields +mojibake (`é`) instead of `é` — any non-ASCII filename is unreachable. +No traversal impact (the `Component` walk at :83-94 rejects `..`; also +verified `%00` fails the read). Related, same file: `resolve_static_path` +uses blocking `is_dir()`/`is_file()` syscalls on the async path +(:97-101) — a convention violation; the read itself correctly uses +`tokio::fs::read`. + +## SRV-09 [minor] — `/openapi.json` error path echoes internals and rebuilds the doc per request + +**Verified:** YES. `src/server/adapter.rs:250-254` — 500 body is +`format!("failed to serialize gateway spec: {e}")` (raw serde error +echoed to unauthenticated callers), and the handler rebuilds + serializes +the whole projection on every request (no caching) — cheap DoS +amplification. Companion finding: `src/adapters/to_openapi.rs:67` uses +`.expect("to_openapi always emits a valid OpenAPI document")` on this +request-triggered path — the only unguarded `expect` reachable from a +route handler (convention violation; also T-11). + +## SRV-10 [minor] — WS upgrade hardcodes `NoCap`; doc claims deployers can pass a stricter policy but there is no injection point + +**Verified:** YES. `src/websocket/upgrade.rs:28-29` says the assembly +layer can pass a stricter channel policy, but `:120` hardcodes +`Arc::new(NoCap)` and `HttpAdapter` offers no parameter. alkcall +documents `NoCap` as the explicit *opt-out* (the default is 256/identity). +One authenticated WS client can open unbounded data channels on one +connection. Minor (post-auth), but the comment misleads and the lever is +missing. Fold-in: the WS upgrade route resolves the bearer token twice +(router-wide `bearer_auth_middleware` + `ws_bearer_auth`) — harmless +today, but a provider with token-use side effects double-counts. + +--- + +# Part B — WebSocket subsystem (byte adapter, upgrade, session) + +## WS-01 [major] — Single demux loop: one dribbled chunk stalls all channels indefinitely + +**Verified:** YES. The WS read task hands whole messages to `AsyncRead` +(`src/websocket/byte_adapter.rs:110-126`); alkcall's demux then does +`read_exact` on header + payload with **no read timeout**, and +`route_payload` awaits the per-channel bounded sender. A peer sends +header `[ch=0][len=16 MiB]` then dribbles one byte per minute: the server +holds the 16 MiB allocation forever and every outstanding channel-0 +call/subscription on that connection hangs — Once-calls only at the 30 s +sweeper, `Sub`/`Pub` pendings (registered `timeout: None` on the client +side) hang until the socket dies. Memory is bounded; availability is not. +There is no read-idle timeout anywhere on the WS path. **Fix:** an idle +timeout on the WS read (close with 1001 on staleness) bounds the stall; +consider it a deployment knob. + +## WS-02 [major] — `Notify::notify_waiters` is lossy — `read_eof` can be missed, in-flight calls hang forever + +**Verified:** YES. `src/websocket/byte_adapter.rs:138-139` (and :319 for +the tungstenite twin) fires `read_eof.notify_waiters()`, which wakes only +*already-registered* waiters and stores no permit. The consumer +(`src/adapters/from_wss.rs:156-166`) spawns its drop-monitor *after* +session setup; if the read task hits EOF before the monitor first polls +`Notified`, the signal is lost. Because `import()` does +`std::mem::forget(session)` (`from_wss.rs:193`), the `close_rx` fallback +never fires either → the monitor never runs `fail_all` → imported-op +calls in flight hang (Once-calls recover only at the 30 s sweeper, if a +sweeper runs at all on this path — see CON-02; `Sub`/`Pub` pendings hang +forever). The module doc at `from_wss.rs:111-113` promises the opposite. +**Fix:** a `watch` channel / `CancellationToken` / permit-storing +`notify_one`, or a checked `AtomicBool`. + +## WS-03 [major] — No data-channel wiring: ADR-067's browser data channels are not implemented + +**ADR drift:** ADR-067 (websocket.md §"Data channels for browsers"), +ADR-048's connection-local overlay contract. +**Verified:** YES. `src/websocket/upgrade.rs:40,62-98` hands the **base** +registry to `install_channel_zero` and runs +`Dispatcher::run_loop_single_stream` over it; there is no +`ChannelCore`/`register_openable`/`ChannelOperations` wiring anywhere in +src/ (grep-verified). A browser can never open a data channel — the exact +capability ADR-067 says the channels design exists to provide; and +ADR-048's bidirectionality (hub calls browser-registered ops via the +connection-local overlay) is unused. This may be a deliberate v1 cut +(the WS tasks scoped channel-0 dispatch only), but the spec promise and +the implementation have not been reconciled in writing — either wire it +or file the OQ / amend the ADR. (Related: the overlay tests in +`tests/ws_overlay_ops.rs` cover the dispatcher's overlay mechanics over +duplex, not browser-opened data channels.) + +## WS-04 [minor] — Write-side chunk parser does no length validation + +**Verified:** YES. `src/websocket/byte_adapter.rs:161-163` (twin at +:343-345) parses the outbound length field with no `MAX_CHUNK_LEN` check +and no error path. Today the only producer is alkcall's mux (capped at 16 +MiB per payload), so the parse always re-syncs; but if any future +producer writes non-chunk-framed bytes, the parser silently waits to +accumulate `8 + len` (up to ~4 GiB) from misaligned offsets — permanent +silent corruption plus a huge `pending` allocation. A one-line validation +that fails the stream would make the invariant loud. + +## WS-05 [minor] — Write-task `pending` buffer is unbounded in bytes (~1 GiB worst case per connection) + +**Verified:** YES. `byte_adapter.rs:143,155` — slot count is bounded (64 +`WriteMsg`s) but bytes are not; each message is up to 16 MiB, so a slow +WS sink plus a large streaming publish buffers up to ≈64 × 16 MiB plus a +partial chunk before backpressure engages. Steady state is small; the +bound is worth a byte cap since `pending` is the only unbounded +accumulator on the write path. + +## WS-06 [minor] — Inbound per-connection memory bound is 64 slots × 64 MiB ≈ 4 GiB + +**Verified:** YES. `READ_SLOTS = 64` bounds messages, not bytes +(`byte_adapter.rs:56`); the plan's "~1 MiB" cap applies only to the write +side (`WS_MESSAGE_CAP`). Inbound WS messages are capped only by +axum/tungstenite's default `max_message_size` (64 MiB); neither +`WebSocketUpgrade::max_message_size` nor `max_frame_size` is configured. +A peer flooding 64 MiB binary messages pins up to ~4 GiB whenever the +demux drains slower than the socket delivers (e.g. during the WS-01 +stall). Set an explicit message-size cap consistent with the plan. + +## WS-07 [minor] — `poll_shutdown` does not close the WS sink; the documented close mapping is not the behavior + +**Verified:** YES. `byte_adapter.rs:269-276` vs the module doc at :25-28 +— `poll_shutdown` drops a **fresh clone** of `write_tx` while the +original stays in the struct, so the channel never closes and the +trailing `ws_sink.close()` doesn't run at shutdown time. In the server +path the teardown cascade still completes (traced end-to-end), so REQ-CH-01 +sentinels do flush, but (a) the Close frame is deferred to an unrelated +cascade step, and (b) any consumer that calls `shutdown()` and keeps the +stream alive (the `from_wss` path does — it forgets its session) never +emits a WS Close frame at all. The comment describes intent, not +behavior. + +## WS-08 [minor] — `_pumps` detached in the server session; no forced-teardown lever + +**Verified:** YES. `upgrade.rs:36` drops `WsPumps` immediately — fine in +the normal path (cascade traced), but `WsPumps::abort()` (the documented +"forced teardown" lever) is never callable on the server path, so a +stuck session (WS-01's dribble) can only be evicted at the TLS/socket +layer outside this crate. + +## WS-09 [minor] — No per-connection or global cap on WS sessions + +**Verified:** YES. The upgrade route has bearer auth (good) but no +concurrency limit, no idle timeout, and each session holds ≥ 4 spawned +tasks plus buffers until the socket dies. Post-auth DoS only, but the +assembly layer cannot add a cap because the built-in route is built +inside `HttpAdapter` (extra routes only add routes; middleware can't wrap +this route from outside). A semaphore in `ws_upgrade_handler` is the +cheap lever. + +## WS-10 [minor] — Dispatcher/mux tasks outlive a failed session task + +**Verified:** YES. `upgrade.rs:65-97` spawns the dispatcher loop +detached; if the session future is dropped/aborted, the spawned tasks +keep the `ChannelManager`/`MuxHandle` alive, which keeps the writer (and +the WS sink) alive — the socket then closes only when the *peer* goes +away. Self-healing in practice; a leak window tied to peer behavior. + +## WS-11 [minor] — ~60 lines of pump logic duplicated between the axum and tungstenite paths + +**Verified:** YES. `byte_adapter.rs:119-180` vs `:299-364` — read task, +write task, and close handling are copy-pasted with only the message +enums differing, while the module doc claims "one implementation, both +directions" (:30-31, :279-284). Any fix (WS-04, WS-06) must be applied +twice; factor the pump bodies over a generic sink/stream of messages. + +## WS-12 [major, cross-crate] — alkcall demux `TooLarge` skip allocates up to 4 GiB from an 8-byte peer header + +**Location:** alkcall `src/channels/adapter.rs:144-148` (`let mut discard += vec![0u8; length as usize];`). **Verified:** YES. Any header with +`length > 16 MiB` reaches this arm; the skip buffer is allocated *before* +reading. Via the WS path this is trivially reachable by any authenticated +browser (8 bytes of header); K connections × `[len = 0xFFFFFFFF]` + +dribble → OOM. Correct sync recovery (verified by alkcall's own resync +test), wrong memory shape — stream-skip with a bounded buffer. Not an +alkhttp defect, but it detonates through `byte_adapter.rs`'s read path; +fix on the alkcall side. + +--- + +# Part C — Gateway (routes, dispatch, error) + +## GW-01 [major] — `/publish` skips `publish_schema` validation that the wire path enforces + +**Verified:** YES. `src/gateway/routes.rs:257-276` parses each NDJSON +line and feeds the stream straight to `invoke_sink`; +`OperationRegistry::invoke_sink` performs only not-found/visibility/ACL/ +handler-kind checks — `publish_schema` validation lives in alkcall's wire +`Dispatcher` only. A Pub op that registers `publish_schema` receives +attacker-controlled arbitrary JSON over HTTP while the same op over the +call protocol aborts invalid chunks. Handlers written against the +validated-wire guarantee get a transport-dependent invariant. **Fix:** +validate in the route (or move validation into the shared +`invoke_sink` spine so both transports enforce it). + +## GW-02 [major] — Per-identity `GET /search` / `GET /schema` responses carry no cache headers + +**Verified:** YES (grep: no `Cache-Control`/`Vary` anywhere in src/). +`/search` and `/schema` are GETs whose bodies are per-caller +(AccessControl-filtered, 200 vs 403 depending on the bearer). With no +`Cache-Control: no-store` (or at least `Vary: Authorization`), shared +caches/CDNs are permitted to serve caller A's authenticated response to +caller B — leaking the op inventory and, compounded with SRV-02, full +schemas. Standard mitigation; matters for any deployment behind a cache. + +## GW-03 [minor] — `INVALID_OPERATION_TYPE` maps to 500 on `/call`//`/batch` but 400 on `/publish` + +**Verified:** YES. `src/gateway/error.rs:17-21` lists only five protocol +codes; `INVALID_OPERATION_TYPE` falls through to `_ => 500` (a +client-fault class reported as server-fault), while `routes.rs:288-297` +maps the identical condition to 400 on `/publish`. Pollutes error-rate +alerting and retry logic; undocumented drift in http-server.md's mapping +table. + +## GW-04 [minor] — SSE error events are not terminal; the stream continues after an `Err` + +**Verified:** YES. `routes.rs:299-313` maps each envelope independently +and keeps going; the documented contract (http-server.md:219-223) and the +wire dispatcher both treat an `Err` as terminal ("the stream ends after +it"). A streaming handler that yields `Err` and then yields again makes +HTTP serve `event:error` followed by more `data:` events, while the same +op over WS emits `call.error` and stops — two transports disagree about +stream semantics. **Fix:** `take_while` on `Ok` (emit the error, end the +stream). + +## GW-05 [minor] — The 30 s `DEFAULT_TIMEOUT` deadline is set but never enforced + +**Verified:** YES. `src/gateway/dispatch.rs:34,165` records a deadline; +nothing on the gateway path ever enforces it (`registry.invoke` does not +wrap the handler in a timeout; the only real timeout lives client-side in +alkcall's pending map). A hung handler holds the HTTP request open +indefinitely. Either enforce (`tokio::time::timeout` around the invoke +for Once ops) or remove the dead metadata. + +## GW-06 [minor] — `/publish` buffers the entire NDJSON body before dispatch + +**Verified:** YES. `routes.rs:187-191` takes `axum::body::Bytes` and +materializes all chunks before `invoke_sink` (:257-266), contradicting +ADR-068 step 4 ("stream each NDJSON line") and the module doc's +disconnect claim (:182-183). Mitigated by the 2 MiB body cap, so memory +is bounded — but there is no true streaming, no backpressure, and a +silent 2 MiB-max publish semantic difference vs the wire Pub path. The +vacuous test at `routes.rs:1572-1583` (see HY-13) implicitly concedes +the buffering. + +## GW-07 [minor] — `Retry-After` is never emitted on any live gateway error path + +**Verified:** YES. The header machinery lives in +`call_error_to_http_response` (`error.rs:61-75`), but the main gateway +error path builds responses by hand (`routes.rs:319-333`) without it; +the only production call site is `not_found_response`, where `retryable` +can never be true. A retryable `HTTP_429`/`HTTP_503` from a handler +reaches callers with **no** `Retry-After`, while http-server.md promises +the mapping. + +## GW-08 [minor] — `/batch` has no cap on the number of operations + +**Verified:** YES. `routes.rs:139-157` dispatches each entry +sequentially; the only bound is the 2 MiB body. A maximal batch of cheap +ops occupies a worker for the sum of all handler latencies (30 s +"deadline" unenforced per GW-05). Also note results are purely +positional — `CallRequest` has no caller-supplied id field. + +## GW-09 [minor] — `/batch` internal-op entries emit `request_id: null` while dispatched entries carry a UUID + +**Verified:** YES. `routes.rs:358-365` vs :335-340 — mixed envelope +shape within one response body; clients correlating on `request_id` see +two shapes. + +## GW-10 [minor] — `/publish` first line without a `chunk` key silently publishes `Value::Null` + +**Verified:** YES. `routes.rs:213` — `unwrap_or(Value::Null)`. A first +line carrying only `{"operation": "…"}` publishes a null chunk 1 instead +of failing with `INVALID_INPUT` (the class of error the route does +reject for a missing `operation` at :216-226). Since null is also a +legitimate payload, client error is indistinguishable from intent. + +## GW-11 [minor] — `/publish` runs four registry lookups + ACL check that `invoke_sink` then repeats + +**Verified:** YES. `routes.rs:240-255` vs alkcall +`registration.rs:289-344` — duplicated enforcement (harmless because +dispatch re-checks and wins, but pure duplication, and inconsistent with +`/call`//`/batch`, which deliberately skip the pre-check and rely on the +registry). + +## GW-12 [minor] — ACL denial on `/subscribe` surfaces as HTTP 200 + SSE `event:error`; on `/call` the same denial is 401/403 + +**Verified:** YES. `subscribe_handler` returns `Sse` (always 200); +pre-dispatch failures (unknown op, internal op, ACL) stream as +`event:error` frames (tests at `routes.rs:1066-1132` assert the 200s). +Defensible per ADR-049 (errors-on-the-stream) but a status-fidelity +asymmetry across the "sole invoke path" that the docs don't call out — +and standard HTTP monitoring never sees auth failures on `/subscribe`. +(See also PRJ-05 — the OpenAPI doc doesn't document this either.) + +## GW-13 [minor] — SSE stream has no keep-alive/heartbeat + +**Verified:** YES. `routes.rs:172` — `Sse::new(stream)` with no +`.keep_alive(...)` and no `retry:` field. Subscriptions are specced as +unbounded, so quiet-but-alive streams are normal state; typical LB/proxy +idle timeouts (30-60 s) will silently terminate them. + +## GW-14 [minor] — Stale module doc: `/publish` claimed to be "a separate module" + +**Verified:** YES. `routes.rs:1-9` says `/publish` (ADR-068) "is a +separate module"; `publish_handler` and all its helpers live in this +same file (:184-297). (Same staleness echoed in AGENTS.md §7's +"5 gateway endpoints" framing — ADR-068 made it 6.) + +--- + +# Part D — Outbound forwarding core + client host + +## FWD-01 [major] — Path-parameter values substituted without percent-encoding; `Url::join` normalizes dot-segments + +**Verified:** YES (empirically). `src/adapters/forward.rs:56-71, +133-141` — `value_to_path_segment` returns the raw string; `Url::join` +then normalizes `..`. Verified: template `/repos/{owner}/{repo}/issues` +with `{owner} = "../../admin"` → `https://api.example.com/admin/...` +(traversal); `{owner} = "a?admin=true"` → the `?` splits the URL and +discards the template tail into query semantics; `#` fragments inject +likewise. A value containing a *later* placeholder also gets expanded by +that later key (iterative substitution over a BTreeMap-ordered input). +Attack: peer-controlled input into a path param escapes a path-scoped +prefix on the upstream (cross-tenant IDOR), **with the operation's +injected credentials attached**. Contrast: the query path *is* correctly +encoded via `query_pairs_mut`. **Fix:** percent-encode each segment +(`utf8_percent_encode` with a path-segment set) before substitution, and +reject/encode `?`/`#`. + +## FWD-02 [major] — `base_url` path prefix silently dropped; no scheme/host validation of the effective URL (SSRF) + +**Verified:** YES (empirically). `forward.rs:74-78` — `Url::join` +resolves against the base *directory*: `base_url = "https://api.openai.com/v1"` ++ template `/chat/completions` → `https://api.openai.com/chat/…` (the +`/v1` is lost). Every test in both adapters uses an origin-only +`base_url`, so the suite can't see it. Worse: a path key that is an +absolute URL (`https://169.254.169.254/…`, `http://localhost:9090/…`) +replaces scheme+host entirely — verified — and reqwest only rejects +non-http(s) *schemes* at send time, so any http/https host is allowed. +Credential injection happens after URL construction, so a +spec-controlled absolute path sends the namespace's credentials to an +arbitrary host. Today specs are assembly-layer-supplied (trusted per +ADR-066), which keeps this out of critical territory — but nothing +enforces that trust boundary (no allowlist, no post-join host-equality +check). **Fix:** require the joined URL to keep the base host (fail +loudly otherwise), require https (or explicit opt-out), and handle the +base-path-prefix case (append to the base path, not the origin). + +## FWD-03 [major] — Custom credential headers follow cross-host redirects (reqwest default policy) + +**Verified:** YES. `src/client/http_client.rs:119-172` never sets +`.redirect(...)`, so reqwest's default (`limited(10)`) applies; its +cross-host scrub removes only `Authorization`/`Cookie`/`cookie2`/ +`Proxy-Authorization`/`WWW-Authenticate` (verified in reqwest 0.13 +source). `HttpAuthScheme::ApiKey { header_name }` credentials +(`forward.rs:110-117`) and all `default_headers` are not in that list. +Attack: an upstream open redirect (or DNS hijack) 302s to an attacker +host → the API key is delivered intact. **Fix:** an explicit redirect +policy — `none`, or a limited same-host policy — for the shared client. + +## FWD-04 [major] — Non-idempotent requests (POST) are retried; retry budget has no total-duration cap + +**Verified:** YES. `http_client.rs:165-167` installs +`RetryTransientMiddleware` with the default strategy, which classifies +5xx/408/429/timeout/connect-error as retryable **regardless of method** +(verified in reqwest-retry 0.9.1; `is_incomplete_message` is explicitly +retried). A POST whose backend committed the write then returned 500 (or +dropped mid-response) is re-sent up to 3 times → duplicate side effects. +Backoff sleeps (default `max_retry_interval` 30 min each) occur outside +reqwest's per-attempt timeout, and `ExponentialBackoff::build_with_max_retries(3)` +has no total-duration cap → total wall time unbounded from the caller's +perspective. **Fix:** skip retries for non-idempotent methods (or make +it a per-adapter policy), and cap total retry duration. + +## FWD-05 [major] — No request/connect timeout by default; `Retry-After` deadlines uncapped → unbounded hangs + +**Verified:** YES. `HttpClientConfig::default()` sets +`request_timeout: None` and no connect timeout exists anywhere +(`http_client.rs`); reqwest applies no timeout unless configured. +`retry_after.rs:27-37` accepts any u64 seconds with no maximum, and +`maybe_sleep_for` sleeps before *every* request to that URL. A hostile +backend answering once with `429` + `Retry-After: 315360000` (~10 years) +stalls every subsequent call to that URL indefinitely. Overflow is safe +(`checked_add` → `None`), and zero/past deadlines are filtered — the gap +is the missing cap and the missing default timeout. **Fix:** default +request timeout (the gateway's 30 s deadline is the natural anchor), a +connect timeout, and a `Retry-After` ceiling. + +## FWD-06 [major] — SSE parser drops events at TCP chunk boundaries and corrupts split multi-byte UTF-8 + +**Verified:** YES (empirically with the verbatim parser). `forward.rs: +409-417` retains only the *last* line of each chunk as `remaining`, +discarding any pending multi-line `data_buffer`; a chunk ending exactly +at `data: …\n` (blank line not yet arrived) silently loses the event +(verified: chunks `"data: {\"n\":1}\n"` + `"\ndata: {\"n\":2}\n\n"` +yield only event 2). Single-chunk delivery (as in the tests) works, +which is why the suite passes. Also `String::from_utf8_lossy` per chunk +(:357) corrupts multi-byte characters split at chunk boundaries → JSON +parse failure → event degraded to a raw string. Plus the trailing +partial line has no length cap (unbounded buffering) and EOF with a +pending event drops it (SSE says dispatch at EOF). For a subscription +forwarder this is silent data loss with no error signal. **Fix:** an +incremental byte-level parser carrying state across chunks. + +## FWD-07 [major] — Vendor JSON content types (`application/*+json`) decoded as per-byte arrays; no response size cap + +**Verified:** YES. `forward.rs:236` — `content_type.contains("application/json")` +misses `application/vnd.api+json`, `application/hal+json`, +`application/problem+json`, which fall to the binary branch and return +`Value::Array` of one `Number` per byte. There is **no response size +limit** on any read path (`response.json()/.text()/.bytes()`), so a +hostile upstream controls caller-side memory. `forward_stream` never +checks content-type at all (a 200 HTML response parses to an empty +stream with no error). **Fix:** match on the mime essence type +(`application/.*+json` suffix semantics), cap response sizes, and treat +non-SSE content on a Sub op as an error. + +## FWD-08 [minor] — Invalid credential values silently produce unauthenticated requests + +**Verified:** YES. `forward.rs:104-126` (and :87-94 for +`default_headers`) — all auth arms drop the header on +`HeaderValue::try_from` failure with no error, no log. A credential with +a control character (or a typo'd `header_name`) means the request goes +out unauthenticated and the caller only sees the upstream's eventual +401. (No leak risk — nothing is logged; verified no tracing calls in the +forwarding path.) A loud error would surface misconfiguration at call +time instead. + +## FWD-09 [minor] — Blocking `std::fs::read` in `build_client` (reachable via public `reload`) + +**Verified:** YES. `http_client.rs:129,144,150` — small-file reads on +the construction path; `SharedHttpClient::reload` (:111-116) is public +and documented as hot-reload, so an async-context caller does blocking +I/O on the async path (convention violation; one-shot small reads, so +impact is a stalled worker). `tokio::fs::read` (or spawn_blocking) is +the cheap fix. + +## FWD-10 [minor] — Upstream error bodies discarded + +**Verified:** YES. `forward.rs:215-227` (and :333-346 for the stream +twin) — on non-2xx the body is dropped; the surfaced message is only +`"HTTP {status}: {reason}"`. Upstream diagnostics (validation details, +rate-limit info) never reach the caller, and unconsumed bodies hinder +connection reuse. The `HTTP_` mapping itself is correct. + +## FWD-11 [minor] — `Retry-After` bookkeeping keyed on pre-redirect URL; eviction order inverted; thundering herd on wake + +**Verified:** YES. `retry_after.rs:127-130` records under `req.url()` +(pre-redirect), so a redirector's rate limit pollutes the origin's entry; +eviction drops the *earliest* deadline (preferentially discarding +soonest-actionable entries while retaining year-long ones, compounding +FWD-05); on expiry all concurrent waiters wake simultaneously (no +jitter). + +## FWD-12 [minor] — Duplicated helpers, dead-ish parameter, non-atomic reload stores + +**Verified:** YES. `value_to_path_segment` and `value_to_query` +(`forward.rs:133-151`) are byte-identical; `forward`'s `op_type` +parameter only toggles `ACCEPT` (if a Sub were ever routed here, the +`text/` branch would buffer the whole SSE stream — currently +unreachable but inviting misuse); `reload` performs two separate +`ArcSwap::store` calls (a reader can observe new config with the old +client); `forward.rs:197` `unwrap_or_else(|_| "null")` masks an +(unreachable) serialization failure by sending a null body. + +--- + +# Part E — from_openapi / from_jsonschema / openapi_spec + +## OAI-01 [major → borderline critical] — Unbounded `$ref` recursion → stack overflow (process abort) on valid, common specs + +**Verified:** YES (empirically). `src/adapters/openapi_spec.rs:199-221` +— `resolve_refs_recursive` recurses with no cycle detection and no depth +budget; a self-referential component (`{"$ref":"#/components/schemas/Node"}` +inside Node — trees, linked lists, cursor pagination: common, *valid* +OpenAPI) recurses until the stack is exhausted. The 128-level parse +limits of serde_json/yaml_serde don't help (each `$ref` hop re-enters +from a fresh clone). Verified empirically: the exact algorithm against a +recursive schema → `thread has overflowed its stack; fatal runtime +error` → abort. Not a catchable panic; `import()` kills the whole +process (startup crash-loop, or remote DoS if specs are ever +runtime-refreshed/peer-supplied). **Fix:** a depth budget + visited set, +erroring cleanly on cycles. + +## OAI-02 [major] — No input-schema enforcement: extra keys become upstream query params; `body` sent verbatim + +**Verified:** YES. `forward.rs:60-72` — every input key that isn't a +path placeholder (and isn't literally `"body"`) is appended as a query +parameter regardless of the registered `input_schema`; neither the +registry nor the adapter validates input against the schema at call +time (ADR-066's "input schema validation before send" is not +implemented anywhere). A composed facade forwards peer input; a peer +adds `"debug": "true"` or `"impersonate_id": "…"` and it reaches the +upstream. Violates the schema-scoped contract `/schema` advertises. + +## OAI-03 [major] — OpenAPI `in: header` / `in: cookie` parameters are silently sent as query parameters + +**Verified:** YES. `openapi_spec.rs:35-40` records `in_` (dead field — +never read); `build_request` has no header-parameter branch. Upstream +auth/trace headers never arrive (confusing failures), and values that +were specified as headers — which don't normally land in upstream +access logs — end up in query strings, which typically do. + +## OAI-04 [major] — Parameter-level and requestBody-level `$ref`s are silently dropped, producing broken operations + +**Verified:** YES. `openapi_spec.rs:236-262` — a parameter entry of the +form `{"$ref": "#/components/parameters/Id"}` (extremely common in real +specs) has no `name`, is silently skipped, and never reaches +`resolve_refs_recursive`; same for `requestBody: {"$ref": …}`. Only +`components/schemas` is indexed (:162-174). Result: the op registers +with `{id}` in the template but `id` absent from the schema; at call +time the placeholder is substituted with the *literal* `{owner}` text +and percent-encoded (`%7Bowner%7D`) — a well-formed request to a +nonsense path, with credentials attached. Silent misbehavior, no error +at import or call time. + +## OAI-05 [minor] — Operation-ID collisions silently overwrite registrations + +**Verified:** YES. `from_openapi.rs:51-65,160` — generated IDs collide +by construction (`/x/{id}/y` and `/x/y` both → `get_x_y`); alkcall's +registry `insert` silently replaces; `import()` doesn't detect +duplicates within its own batch. One op silently shadows another; +`/search` shows fewer ops than the spec declares. + +## OAI-06 [minor] — Unsupported OpenAPI features degrade silently + +**Verified:** YES. `"default"`/wildcard response keys become +`ErrorDefinition { code: "HTTP_0", http_status: None }` — entries that +never match a real status (`from_openapi.rs:142-145`); `trace` ops are +silently skipped (`openapi_spec.rs:12-13`); `servers` overrides, +parameter `style`/`explode`, and array/object query serialization are +unsupported and silent (arrays become `"[1,2]"`); a `default`-declared +SSE stream is missed by `detect_op_type` and would return one giant +text string. None fail loudly at import; each produces an op that +misbehaves only at call time. + +## OAI-07 [minor] — Magic `"body"` input key collides with spec parameters named `body` + +**Verified:** YES. `from_openapi.rs:96-102` + `forward.rs:66-67` — a +declared parameter named `body` is overwritten by the requestBody schema +and diverted to the request body at call time, unreachable as the +query/path/header parameter the schema advertised. + +## OAI-08 [minor] — Guarded `expect`s in `openapi_spec` lib code + +**Verified:** YES. `openapi_spec.rs:144,170` — `expect("paths is object")` +etc., each immediately preceded by an `is_object()` check (unreachable +today, but AGENTS.md §2 says no `expect` outside tests; `if let` costs +nothing). + +## OAI-09 [minor] — `from_jsonschema` defers all config validation to call time + +**Verified:** YES. `from_jsonschema.rs:36-51` — malformed `method`/ +`path_template`/`base_url` surface as `CallError::internal` on first +invoke rather than at construction. Also doc drift: the module doc says +"`Internal` by default" (:8) but the adapter passes the caller's +`OperationSpec` through verbatim — `from_openapi` hardcodes +`Visibility::Internal` (:169, tested); `from_jsonschema` does not. + +--- + +# Part F — Projections (to_openapi, to_mcp) + +## PRJ-01 [major] — `/search` 200 response schema is wrong twice (missing envelope wrapper, wrong item fields) + +**Verified:** YES. `to_openapi.rs:100,335-351` documents +`{operations: [{name, description}]}`; the actual body is the envelope +wrapper (`{request_id, result, output}` — proven by the routes tests +reading `body.output`) and items carry `name`/`namespace`/`op_type` with +**no** `description`. The summary "Returns names + descriptions" is +false. Clients generated from this doc are broken on day one. + +## PRJ-02 [major] — `/schema` 200 response schema is wrong (missing wrapper; omits `op_type`/`visibility`/`access_control`/`channel_open`/`publish_schema`) + +**Verified:** YES. `to_openapi.rs:124,353-364` documents the bare spec; +the runtime body is the envelope with `spec_to_json`'s fuller inner +object. Same wrapper-miss as PRJ-01. + +## PRJ-03 [major] — Documented 400 `INVALID_INPUT` doesn't match the runtime mapping (422) or axum's rejection bodies + +**Verified:** YES. The doc maps 400 for `/call`//`/batch`//`/schema`; +the runtime mapper is `INVALID_INPUT → 422` (`error.rs:51`), and axum's +extractors reject malformed bodies with plain-text 400/415/422 bodies +that have none of the documented `{code, message, retryable}` shape. +The doc has no 422 response at all. `/publish` *is* correct here — the +inconsistency between two pages of the same doc underlines the drift. + +## PRJ-04 [major] — Operation-level error statuses projected under statuses the runtime never produces + +**Verified:** YES. `to_openapi.rs:544-563` projects an op error +declared at 429 under a `429` response; the runtime mapper is purely +code-driven and `ErrorDefinition.http_status` is never consulted at +runtime — a `RATE_LIMITED` error surfaces as **500** unless the code is +literally `HTTP_429`. A consumer building backoff logic around the +documented 429 gets 500s. The test `operation_errors_projected_onto_call` +enshrines the wrong behavior. Either project non-`HTTP_*` codes under +500, or honor `http_status` at runtime. + +## PRJ-05 [major] — `/subscribe` documented HTTP error statuses are structurally unreachable; in-band SSE errors undocumented + +**Verified:** YES. `subscribe_handler` always returns 200 + SSE +(GW-12); unknown op, internal op, ACL denial, and handler errors are all +`event:error` frames (asserted by tests). The doc claims full +protocol-status responses and never mentions `event: error` frames; a +client built from it waits for a 404 that will never come and hangs +reading the stream. + +## PRJ-06 [major, security] — MCP `schema` tool skips the per-operation AccessControl check that HTTP `/schema` performs + +**Verified:** YES. `to_mcp.rs:148-174` dispatches straight to +`services/schema` with no pre-check (compare `routes.rs:125-127`); the +handler returns the full spec — including `access_control.required_scopes` +— for any registered op the caller is forbidden to call, including +unauthenticated callers. An unprivileged MCP client reads the exact +scopes needed for escalation. Per-caller filtering is asymmetric between +the two projections of the same dispatch spine; the test +`schema_returns_full_operation_spec` enshrines the leak (fetches with +`identity: None` and asserts `access_control` is present). + +## PRJ-07 [major] — MCP `search` tool advertises a `query` filter it silently ignores + +**Verified:** YES. The input schema advertises "Optional substring +filter" (`to_mcp.rs:56-59`) but `call_tool` drops `arguments` entirely +for search (`:382`) — an LLM passing `{"query": "fs"}` receives the full +unfiltered listing (wasted context; in large registries, context +overflow). + +## PRJ-08 [major] — MCP `search` does not exclude `Pub` operations, contradicting ADR-068 + +**Verified:** YES. The filter is `!matches!(op_type, "sub" | +"subscription" | "Sub")` (`to_mcp.rs:268-274`); ADR-068 says to_mcp +"excludes both `Sub` and `Pub`". A discovered Pub op can never be +invoked via the `call` tool (`invoke` on a Sink returns +`INVALID_OPERATION_TYPE`) — the advertised discovery surface is a lie +for the entire Pub class. (The `"subscription"`/`"Sub"` match arms are +dead — `op_type_str` only emits lowercase.) + +## PRJ-09 [minor] — MCP `batch` item shape contradicts its own tool description + +**Verified:** YES. `call` returns the raw output as `structuredContent`; +batch items are `{"isError": …, "output"|"error": …}` — the description +claims "each shaped like a `call` result". False on both success and +error shapes. + +## PRJ-10 [minor] — `structuredContent` emitted as non-object values (MCP spec says object) + +**Verified:** YES. `to_mcp.rs:293-298` passes the operation output +through verbatim — a string/array/null output produces a non-object +`structuredContent`, and batch returns a top-level array. rmcp accepts +it; strict MCP clients may not (client impact unverified). + +## PRJ-11 [minor] — `expect`/`unwrap` in `to_openapi` library code + +**Verified:** YES. `to_openapi.rs:67` (see SRV-09 — the reachable one), +`:440` (guarded by `is_object`), `:505,512` (guarded by `len() == 1`). +All currently unreachable-in-practice; convention violation. + +## PRJ-12 [minor] — Generated doc is nondeterministic when the same error code is declared with different statuses + +**Verified:** YES. `collect_operation_errors` dedupes by code while +iterating a `HashMap` — two regenerations of `/openapi.json` from +identical registry state can differ in which status wins (SipHash +randomized per process). Spec-diffing consumers see phantom changes. +Dedupe by `(code, status)` or sort. + +## PRJ-13 [minor] — Hand-rolled MCP argument errors omit `retryable`, diverging from the `CallError` wire shape + +**Verified:** YES. `to_mcp.rs:159-163,200-204,230-234` — the structured +errors lack `retryable` (required by the OpenAPI error schemas and +always present on `CallError`); a non-string `operation` reports the +misleading "missing required field: operation". + +## PRJ-14 [minor] — `components.schemas` defined but never referenced; inline duplication guarantees drift + +**Verified:** YES. `to_openapi.rs:88-91,534-542` — `schema_call_request()` +is inlined verbatim 4× and also emitted into components; nothing uses +`$ref`. Shape changes must be edited in lockstep or the doc +self-contradicts. + +## PRJ-15 [minor] — `/search` documents 401/403 that cannot occur, omits the 404 that can; no `securitySchemes` anywhere + +**Verified:** YES. `services/list` has default ACL, so `/search` returns +200 even unauthenticated (per-op filtering happens inside the listing — +tested); the doc's 401/403 for `/search` can't occur. And the doc +declares no `securitySchemes` despite Bearer being the contract +(ADR-004) — generated clients won't know to authenticate. + +--- + +# Part G — Consumer adapters (from_wss, from_mcp) + +## CON-01 [major] — `from_mcp` imports only the first `tools/list` page + +**Verified:** YES. `from_mcp/mod.rs:86` — a single `list_tools` call; +`next_cursor` is never followed. rmcp 1.8 provides +`list_all_tools()` for exactly this. Any server with enough tools to +paginate silently truncates; no error, no log. One-line fix. + +## CON-02 [major] — `from_wss` drop monitor is one-shot and racy; calls racing the drop hang forever + +**Verified:** YES. Two code-verified gaps beyond WS-02's lost +notification: (1) the monitor runs `fail_all` exactly once and exits — +a call whose pending entry is registered *after* `fail_all` ran is +never resolved, and no pending-entry sweeper exists on the client path +(alkcall's sweeper runs only inside `Dispatcher::run_loop`, which +`from_wss` never takes; the module doc's "no hang until the 30s sweeper +deadline" promise is false on this path); (2) the race window above. +The test `connection_drop_fails_in_flight_calls_retryable_no_hang` +covers only calls registered *before* the drop. **Fix:** a `watch` +channel (WS-02) plus a periodic sweep of the pending map while the +session lives. + +## CON-03 [major] — `from_wss` accepts plaintext `ws://` and sends the Bearer token over it + +**Verified:** YES. `from_wss.rs:114-128` — `IntoClientRequest` accepts +any scheme; nothing enforces `wss://` and the Authorization header is +attached unconditionally. The crate's own tests dial `ws://`. A config +typo (`ws://prod-node/alk/channels`) silently ships a long-lived bearer +credential over plaintext. (TLS validation itself is fine — +rustls-tls-webpki-roots, no danger options anywhere.) **Fix:** refuse +`ws://` when a token is present (or unconditionally unless explicitly +allowed). + +## CON-04 [minor] — `content_block_union_schema` audio variant requires a non-existent `"audio"` property + +**Verified:** YES. `from_mcp/mod.rs:240-248` — the property is `data` +but `required` lists `audio`; valid audio blocks never satisfy the +schema. Published as `output_schema` and inside `error_schemas`; the +variant test checks only the enum tags. Consumers validating handler +output against the declared schema reject valid audio results. + +## CON-05 [minor] — `from_mcp` per-call capability read is dead code; module doc claims per-call credential use + +**Verified:** YES. `mod.rs:139-143` computes the token's length and +discards it; the credential actually used is the transport-pinned +import-time token (rmcp's config is immutable post-construction). An +integrator injecting per-call/per-user tokens into +`OperationContext.capabilities` gets silence. Doc + dead read should be +corrected to "import-time credential". + +## CON-06 [minor] — 401 classification by substring match on `Debug` output + +**Verified:** YES. `mod.rs:103-116` — `format!("{error:?}").contains("401")` +misclassifies transport errors whose URL contains `:4010/` (port 4010) +as `Unauthorized`, and misses differently-worded auth failures. +Fragile in both directions; affects only the surfaced error variant. + +## CON-07 [minor] — `from_wss` doc claims imported handlers read per-call credentials; they carry no capabilities at all + +**Verified:** YES. `from_wss.rs:7-9` vs alkcall `from_call.rs:141-148` +— bundles register with `Capabilities::new()`; only the dial-time token +ever authenticates. Same class of doc-vs-reality drift as CON-05 +(ADR-014's chain is respected, but the doc describes an implementation +that doesn't exist). + +## CON-08 [minor] — `std::mem::forget(running)` leaks the rmcp session — no teardown, no DELETE, SSE stream left open + +**Verified:** YES. `mod.rs:98` — the fire-and-forget pattern skips +rmcp's session teardown; each `import()` leaves an open server-side +session + long-lived GET SSE stream until the remote times it out. +Repeated imports accumulate. No close path exists on `FromMCP`. + +## CON-09 [minor] — `from_wss` import leaks the whole session by design; no shutdown handle for reconnect scenarios + +**Verified:** YES. `from_wss.rs:189-193` — deliberate (ADR-070 v1), +but a reconnecting assembly layer calling `import()` again stacks a +second full session with duplicate op names and no way to tear the +first down. Worth an ADR-070 note or a `close()`/handle API in v1.1. + +## CON-10 [minor] — `[[test]] full_surface` missing `test-support` in `required-features` → `cargo test --features mcp` fails to compile + +**Verified:** YES (empirically). `Cargo.toml:73-75` vs +`tests/full_surface.rs:26` importing test-support-gated items +(`websocket/mod.rs:23-24`). `cargo test --features mcp` fails with +unresolved imports; masked because CI uses `--all-features`. + +## CON-11 [minor] — Transport-level `tools/call` failures map to undeclared `INTERNAL` + +**Verified:** YES. `mod.rs:149-152` — remote down/timeout/JSON-RPC +error → `CallError::internal`, which is not in the declared +`error_schemas` (only `MCP_TOOL_ERROR` is); MCP JSON-RPC error codes are +flattened, losing fidelity the rest of the crate maintains via +`HTTP_`. + +## CON-12 [minor] — Remote tool names interpolated into op names without sanitization + +**Verified:** YES. `mod.rs:169-171` — a remote-controlled `tool_name` +containing `/` yields a three-plus-segment op name, breaking the +two-segment `ns/op` convention the gateway and namespace reasoning +assume. (Registry acceptance of such names unverified — if it rejects, +downgrades to cosmetic.) + +## CON-13 [minor] — Auth tokens held as plain `String` in adapter builders + +**Verified:** YES. `from_wss.rs:48`, `from_mcp/mod.rs:37` — +`auth_token: Option` with a public accessor; alkcall's +`Secret` zeroizing wrapper is used correctly at the +registration boundary but the builder-held copy is unguarded plaintext. +No `Debug` derives exist, so accidental logging is unlikely — but +holding `Secret` would match the crate's own posture. + +--- + +# Part H — Hygiene / cross-cutting + +## HY-01 [major] — `openapiv3` is a production dependency used only by a test + +**Verified:** YES. `Cargo.toml:44` vs sole usage `to_openapi.rs:1103` +(inside `#[cfg(test)]`). Every consumer compiles `openapiv3` (and its +tree) for nothing. Move to `[dev-dependencies]`. + +## HY-02 [major] — 110 missing-docs warnings under `-W missing_docs` + +**Verified:** YES (ran with `RUSTDOCFLAGS=-W missing_docs`). Default +`cargo doc` is 0-warning only because `missing_docs` is off. Worst: +`openapi_spec.rs` (28), `http_client.rs` (28), `forward.rs` (10), +`server/adapter.rs` (10), `dispatch.rs` (7), `routes.rs` (6); +`gateway/mod.rs` and `server/mod.rs` lack module docs. For a crate +prepping crates.io, this is the largest single hygiene gap. + +## HY-03 [minor] — Guarded `expect`/`unwrap` spots that would panic silently if their guard is edited + +**Verified:** YES. `to_openapi.rs:440` (guarded by `is_object`), +`:505,512` (guarded by `len() == 1`), `openapi_spec.rs:144,170` +(guarded by `is_object`). Safe today; the pattern is one edit away from +a panic. (The unguarded ones are SRV-09/PRJ-11.) + +## HY-04 [minor] — `unwrap` in shipped `test_support` + +**Verified:** YES. `upgrade.rs:161` — `serde_json::to_vec(envelope).unwrap()` +inside `pub mod test_support`, which ships behind the opt-in +`test-support` feature. Documented as intentional; note it is published +API surface with a panic inside. + +## HY-05 [minor] — Unused dependencies and feature slack + +**Verified:** YES (grep + build). `bytes` declared, zero direct use in +src/ (the only `Bytes` is axum's re-export); `parking_lot` declared, +zero direct use (obtained transitively via alkcall); `tokio` features = +`["full"]` drags in `process`/`signal` extras. All cheap to prune. + +## HY-06 [minor] — `HttpClientConfig.retry_policy` exposes `reqwest_retry::ExponentialBackoff` in the public API + +**Verified:** YES. `http_client.rs:34` — a semver anchor to an upstream +concrete type and an awkward construction surface; consider an owned +config struct. + +## HY-07 [minor] — `READ_SLOTS` is `pub` in a public module (implementation constant leak) + +**Verified:** YES. `byte_adapter.rs:56` — reachable as +`alkhttp::websocket::byte_adapter::READ_SLOTS` but deliberately not +re-exported; either privatize or document it as API. + +## HY-08 [minor] — `tokio-tungstenite` declared three times with different feature sets + +**Verified:** YES. `wss` (:19), `test-support` (:20), and the +dev-dependency (:59) each pull it; feature unification currently lands +on the superset, but `test-support` extending `wss` would remove the +fragility. + +## HY-09 [minor] — `8 + len` can overflow on 32-bit targets + +**Verified:** YES. `byte_adapter.rs:163,345` — `len` is a parsed +`u32 as usize`; `8 + u32::MAX` overflows in debug on 32-bit. Saturating +add (or the WS-04 validation) removes it. + +## HY-10 [minor] — Stale/contradictory docs + +**Verified:** YES. `from_wss.rs:86-88` says dropping the session tears +the WS down "(via `WssSession::drop` **or `std::mem::forget`**)" — +`mem::forget` prevents `Drop` and does the opposite (the comment at +:189-192 says so); `routes.rs:1-9` (GW-14); `[ADR-051]: +https://docs.rs/alkhttp` placeholder link definitions and a relative +filesystem link in `websocket/mod.rs:2` break on docs.rs. + +## HY-11 [minor] — The whole `docs/` tree (672K) ships in the package + +**Verified:** YES. The `exclude` list omits `docs/reviews/` but keeps +`docs/architecture/`; fine if intentional (ADR links), worth a decision +before publish. + +## HY-12 [minor] — Duplicate dependency roots + +**Verified:** YES (`cargo tree -d`). `getrandom 0.3/0.4` + +`cpufeatures 0.2/0.3`, driven by `tokio-tungstenite 0.28` here vs +`tungstenite 0.29` via axum's ws. Aligning tokio-tungstenite with axum's +tungstenite collapses the duplicates. + +## HY-13 [minor] — Vacuous test: `publish_body_is_fully_consumed_before_dispatch_not_required` + +**Verified:** YES. `routes.rs:1572-1583` — an empty test body (comment +only) that passes vacuously; its own comment cites an +"infra-integration-suite" socket-level test that does not exist in +`tests/`. Wire the socket-level early-disconnect test or delete the +stub. + +--- + +# Part I — Coverage (cargo-llvm-cov, all features) + +Overall: **93.86% lines / 91.75% functions / 94.23% regions** — high, +but the uncovered residue concentrates exactly where the findings live. + +## COV-01 — `adapters/forward.rs` 82.3% lines / 67.9% functions (lowest in the crate) + +Uncovered regions (JSON export): the binary-response branch (247-264), +the Basic-auth arm (122-125), the ApiKey arm (116-119), the +`value_to_*` helpers (137-151), non-2xx error mapping branches +(205-213), the SSE stream error path (374-383), and the `+json`/ +content-type branches (239-244). These are precisely FWD-01/03/07/08/10 +— the highest-risk code in the crate is also its least-tested. + +## COV-02 — `client/http_client.rs` 81.4% lines + +Uncovered: the mTLS/CA-bundle build paths (133-164) and the `Debug` +impl (87-95). No test constructs a client with a CA bundle or client +cert; FWD-03 (redirect policy) has no test because no redirect test +exists. + +## COV-03 — `websocket/byte_adapter.rs` 82.2% lines + +Uncovered: `WsPumps::abort`/`read_eof` (91-108 — the WS-02 mechanism is +untested), the tungstenite pump twins (309-337 — the duplicated code is +untested by the axum-path tests), and the write-side split/drain edge +paths. A tungstenite-path test suite (or dedup, WS-11) is needed. + +## COV-04 — `server/decoy.rs` 87.1% lines + +Uncovered: `percent_decode` (112-122 — SRV-08's bugs are untested), +`hex_digit`/`mime_for_path` (129-154), and static-site serving +(129-154). The traversal guards *are* tested; the decoding is not. + +## COV-05 — `gateway/dispatch.rs` 91.0% lines + +Uncovered: the capability-inheritance path (52-64) and error branches +(202-211, 270-273). + +## COV-06 — `server/adapter.rs` 91.9% lines + +Uncovered: the mcp router arm (127-132 — never exercised without the +feature in unit tests), `ProtocolHandler::handle`/ALPN entry (192-196, +228-230), the h2 enable-connect branch (235-243), and the +`/openapi.json` error path (250-256 — SRV-09 untested). + +## COV-07 — `server/state.rs` 62.5% lines + +Uncovered: `FromRef for Arc` (49-52) +— only the decoy FromRef is tested. + +## COV-08 — Semantic test gaps (decided behaviors with no test) + +The high line-coverage hides the gaps that let the findings land: + +1. `/schema` on an `Visibility::Internal` op (SRV-02) — the four other + routes are tested; schema is not. +2. `extra_routes` auth layering (SRV-01) — no test mounts an extra + route and asserts identity resolution. +3. `/mcp` body limit (SRV-03). +4. SSE across real TCP chunk boundaries (FWD-06) — the parser is only + tested with single-chunk delivery. +5. Cross-host redirect behavior with custom credential headers + (FWD-03). +6. Retry policy on a non-idempotent op (FWD-04). +7. Recursive `$ref` specs (OAI-01 — a test importing a self-referential + spec would have caught the abort immediately). +8. Absolute-URL path templates / base-path-preserved base URLs + (FWD-02). +9. `/publish` with `publish_schema`-registered ops (GW-01). +10. `from_wss` drop *while a call is being registered* (CON-02's race). +11. `from_mcp` paginated `tools/list` (CON-01). +12. The tungstenite pump path end-to-end (COV-03). +13. MCP `search` with a `query` argument (PRJ-07); MCP `search` + containing Pub ops (PRJ-08); MCP `schema` ACL denial (PRJ-06). + +--- + +# Cross-cutting: what is solid (verified) + +Listed so the remediation plan can focus on what's actually broken: + +- **Auth enforcement on the default surface.** Every gateway route + + `/openapi.json` + `/healthz` sits under `bearer_auth_middleware` + (adapter.rs:154-173, applied before merges of the *default* router); + the WS route carries the fail-closed `ws_bearer_auth` (401 before + upgrade, tested). Dispatch re-runs visibility + ACL with + `internal: false` and `forwarded_for: None` hardwired — wire callers + cannot spoof forwarding or escalate to internal (verified in alkcall's + registry code, defense in depth regardless of entry point). +- **No-env-vars invariant (ADR-014).** Zero `std::env::var` reads in + src/; the only occurrences are negative tests asserting env is + ignored. Credentials flow exclusively + `context.capabilities.get(namespace)` → `expose_secret()` → header. +- **Credential hygiene.** No tracing calls anywhere in the forwarding + path (nothing logs headers/tokens); `Secret` zeroizing used at + the registration boundary; header names/values pass through + `HeaderName/HeaderValue::try_from` (CRLF injection structurally + prevented — the failure mode is silent drop, not injection); no + `danger_accept_invalid_certs` anywhere; stdio transport unreachable + for MCP (rmcp feature set excludes child-process). +- **Internal-op invisibility on /call, /batch, /subscribe, /publish** + (pre-checks + registry re-checks; `/call` on Internal → 404, tested) — + the gap is `/schema` (SRV-02) only. +- **Body limits on axum-extracted routes.** `/call` `/batch` + `/subscribe` `/publish` are capped at 2 MiB by axum's default (crate + never disables it); JSON depth bombs rejected by serde_json's + recursion limit; YAML bombs bounded by yaml_serde's alias/depth + limits (verified against yaml_serde 0.10.7 source). +- **`/publish` terminal-error shape matches ADR-068** (plain HTTP + status + JSON body, never an NDJSON line — tested for 200/400/401/ + 403/404/500); OQ-02 framing implemented as decided. +- **Batch semantics:** per-entry envelopes, one bad op doesn't fail the + batch, order preserved (tested). +- **WS framing core is correct.** Byte-stream treatment both + directions; split headers/payloads reassemble (read_exact-based + demux); oversized chunks split legally across messages (16 MiB + round-trip test); WS Close *and* abnormal termination → EOF → all + channels cleared (traced end-to-end; `disconnect_mid_call` test); no + busy-wait (the POC spin was replaced with `poll_ready`); text message + → 1002 close. Identity cannot be escalated via client-supplied + `auth_token` on channel 0 (NoopProvider fallback verified in alkcall). +- **WS upgrade auth is header-only, fail-closed, and non-WS requests + cannot reach the session.** No per-request token in URLs anywhere. +- **Error mapping basics (ADR-023).** Protocol codes → statuses per the + documented table (incl. FORBIDDEN → 401-without-identity/403-with); + `HTTP_` parsing with malformed-code → 500 fallback (tested); + unknown codes → 500, never panic. +- **Producer/consumer naming, module structure, feature matrix.** No + server/client framing in API names; one module per file, all + re-exported; `--no-default-features`, `mcp`-only, `wss`, and + `--all-features` all compile; clippy clean in both modes; all 19 + cited ADR numbers resolve to real files. +- **No panics/locks-across-await/blocking-I/O on the hot paths.** The + exceptions are individually filed (SRV-09, HY-03/04, FWD-09, + SRV-08's `is_dir`); locks that exist are parking_lot or + poison-safe std locks per convention. + +--- + +# Remediation plan + +Ordered by dependency and severity; each unit is independently +shippable with its own acceptance gate. The overarching gate mirrors the +alkcall review lesson: **each unit's acceptance gate is the test that +would have caught its worst finding.** + +## Unit 1 — Auth/visibility at the seams (SRV-01, SRV-02, SRV-03, PRJ-06) + +The highest-value unit; all four are small, mechanical, and +security-bearing. +- SRV-01: apply the auth layer after merging extra routes (keep + per-route opt-out possible by documenting that extras may carry their + own layers); add a test mounting an extra route asserting identity is + resolved (and one for the explicit opt-out shape). +- SRV-02: add the `is_internal_op` → 404 guard to `schema_handler` + (mirror `/call`); test `/schema?name=` → 404 + unauthenticated and for unauthorized identities. +- SRV-03: body-limit the `/mcp` nest (explicit `DefaultBodyLimit`); + test a large body → 413. +- PRJ-06: run the same `access_check_for_op` pre-check in the MCP + `schema` tool (or filter the returned spec per identity); fix the + enshrining test. + +**Gate:** the four new tests above; full suite green. + +## Unit 2 — Outbound request construction + retry/timeout policy (FWD-01..05, OAI-01..04, FWD-11) + +The largest unit; mostly inside `forward.rs`/`http_client.rs`/ +`openapi_spec.rs`. +- FWD-01: percent-encode path segments; reject/encode `?`/`#`; iterate + substitution safely (single-pass template rendering, not iterative + replace). +- FWD-02: post-join host-equality validation (fail loudly on host + change), scheme allowlist (https default), and correct base-path + appending (fix `Url::join` directory semantics). +- OAI-01: depth budget + visited set in `resolve_refs_recursive`; + clean error on cycles. **Acceptance gate: an import test with a + self-referential schema returns an error instead of aborting.** +- OAI-02: enforce `input_schema` at call time (reject undeclared keys, + or document the pass-through as explicit), implement ADR-066's + validate-before-send for `from_jsonschema`. +- OAI-03/OAI-04: honor `in: header`; resolve parameter/requestBody + `$ref`s (index `components/parameters` + `requestBodies`); error + loudly on unresolved path placeholders instead of sending literal + `%7Bowner%7D`. +- FWD-03: explicit redirect policy (none or limited same-host). +- FWD-04: no retries for non-idempotent methods (or per-op idempotency + config); cap total retry duration. +- FWD-05: default request + connect timeouts (anchor to the gateway's + 30 s), cap `Retry-After` (e.g. 300 s ceiling, configurable). +- FWD-11: key `Retry-After` on the effective URL; evict + longest-deadline-first (or soonest); add jitter. + +**Gate:** tests for each bullet — traversal value, absolute-URL +template, redirect-with-API-key, POST-retry, recursive-$ref import, +header-param, `$ref` parameter, timeout default. + +## Unit 3 — Gateway correctness (GW-01, GW-03..GW-13) + +- GW-01: `publish_schema` validation in the shared spine or the route. +- GW-03: map `INVALID_OPERATION_TYPE` → 400 (or 422) consistently; + document in http-server.md. +- GW-04: SSE error events terminal (`take_while` Ok / emit-and-end). +- GW-05: enforce or remove the 30 s deadline. +- GW-06: either stream the NDJSON body (axum `Body` → framed stream) or + document the 2 MiB buffered semantic in ADR-068. +- GW-07: route error responses through `call_error_to_http_response` + so `Retry-After` fires. +- GW-08: cap batch size (constant, e.g. 100). +- GW-09: generate request ids for internal-op batch entries. +- GW-10: reject a first line missing `chunk` with `INVALID_INPUT`. +- GW-11: drop the redundant `/publish` pre-checks (mirror `/call`). +- GW-12/GW-13: keep 200-on-stream (ADR-049) but document it in the + projection (PRJ-05) and add SSE keep-alive. +- GW-14: fix the stale module doc. + +**Gate:** a `/publish` test with a `publish_schema`-registered Pub op +rejects an invalid chunk; SSE error-terminal test; batch-cap test. + +## Unit 4 — WS/adapter robustness (WS-01..WS-11, HY-09) + +- WS-02 (do first): replace `notify_waiters` with a `watch` channel or + permit-storing signal; CON-02 falls out of this + a pending-map + sweep. +- WS-01: idle timeout on the WS read path (configurable; default + bounds the dribble stall). +- WS-04/HY-09: validate `len > MAX_CHUNK_LEN` on the write side + (saturating add). +- WS-05/WS-06: byte-based caps — cap `pending` growth and set an + explicit inbound `max_message_size` consistent with the plan's ~1 MiB + intent. +- WS-07: make `poll_shutdown` actually drop the held sender so the + documented close mapping holds. +- WS-08/WS-09/WS-10: keep `_pumps` handle for forced teardown; add a + session semaphore; document the detached-task semantics. +- WS-11: dedup the pump bodies (single generic implementation; also + fixes COV-03). +- Cross-crate: file/land the alkcall demux fix for WS-12 (stream-skip + instead of `vec![0u8; length]`). + +**Gate:** a from_wss test that drops the connection mid-registration +(CON-02); a dribble-stall test bounded by the idle timeout; the +tungstenite path covered by shared tests. + +## Unit 5 — Consumer adapter fixes (CON-01..CON-13) + +- CON-01: `list_all_tools()`. +- CON-03: refuse `ws://` with a token (or unconditionally with an + explicit escape hatch). +- CON-04: fix the audio variant `required` list. +- CON-05/CON-07: correct the doc claims (import-time credential) and + remove the dead capability read. +- CON-06: classify on typed error variants where rmcp exposes them; + fall back to substring only as a last resort. +- CON-08/CON-09: add explicit close/teardown handles (or document the + leak + ADR-070 note). +- CON-10: add `test-support` to `full_surface`'s `required-features`. +- CON-11/CON-12/CON-13: declared error codes for transport failures; + sanitize/validate remote tool names; `Secret` for held + tokens. + +**Gate:** `cargo test --features mcp` compiles and passes (CON-10); +pagination test against a paginating MCP server. + +## Unit 6 — Projection fidelity (PRJ-01..PRJ-05, PRJ-07..PRJ-15) + +The `to_openapi` document needs a systematic diff against the runtime +(routes tests are the oracle): envelope wrappers (PRJ-01/02), the +422-vs-400 mapping (PRJ-03), error-status honesty (PRJ-04), the +`/subscribe` 200+in-band-error contract (PRJ-05), `securitySchemes` +(PRJ-15). MCP: honor/drop the `query` filter (PRJ-07 — honoring is +trivial), exclude Pub (PRJ-08), fix the batch doc (PRJ-09), wrap +non-object outputs or document (PRJ-10), add `retryable` (PRJ-13), +deterministic error merge (PRJ-12). + +**Gate:** a test that generates the doc and asserts it against a +hand-written golden file matching the routes tests' actual bodies; +MCP search respects `query` and excludes Pub. + +## Unit 7 — Hygiene + coverage (HY-01..HY-13, COV-01..07, SRV-04..SRV-10, OAI-05..OAI-09, FWD-08..FWD-10, FWD-12) + +Mechanical, parallelizable cleanup: +- HY-01 (`openapiv3` → dev-deps), HY-05 (unused deps), HY-08 (feature + triplication), HY-11/HY-12. +- HY-02: `missing_docs` sweep (110 warnings) before crates.io. +- SRV-05..SRV-10, OAI-05..OAI-09, FWD-08..FWD-10, FWD-12: per-finding + small fixes. +- COV-01..07: backfill the uncovered regions identified in Part I, + prioritizing the forwarding-core error/binary/auth paths and the + tungstenite pump path. +- HY-13: wire or delete the vacuous `/publish` test. + +**Gate:** `cargo doc -W missing_docs` clean; `cargo tree -d` reduced; +coverage of `forward.rs` ≥ 95% lines. + +## Suggested sequencing + +``` +Unit 1 (auth seams) → no deps; do first (smallest, highest value) +Unit 3 (gateway) → no deps; independent +Unit 2 (outbound requests) → no deps; largest; can start in parallel +Unit 5 (consumer adapters) → no deps; small +Unit 4 (WS robustness) → depends on nothing in-tree; coordinate WS-12 with alkcall +Unit 6 (projections) → after Unit 3 (shares routes.rs/error mapping docs) +Unit 7 (hygiene/coverage) → continuous; land the dep/doc items any time +``` + +Units 1, 3, 5 touch disjoint files and can proceed in parallel. Unit 2 +and Unit 4 both touch `byte_adapter.rs` only via HY-09 (trivial). +Unit 6's doc-golden work should follow Unit 3 so the runtime contract +is settled before it is documented. + +## On the baseline objective + +The extraction is structurally sound: the architecture held, the wire +formats are right, the no-env-vars and Internal-by-default invariants +are real and tested, and the end-to-end integration suites (the thing +alkcall's review found missing) exist and are substantive. What needs +attention is the *edge* behavior: the seams where the default surface +meets extension points (extra routes, `/schema`, MCP), the outbound +forwarding path (URL construction, redirects, retries, timeouts, SSE +parsing), and the projections' fidelity to the runtime they describe. +None of these block the crate's core function; all of them are the kind +of thing that surfaces as a production incident rather than a failed +test. **Treat Unit 1 and Unit 2 as the gate for any deployment-facing +milestone.** + +--- + +## Verification log (this pass) + +- All findings carry `file:line` references verified against tree + `4a825d3` by the subsystem passes; the consolidating pass + re-verified: SRV-01/SRV-02 (adapter.rs:170-183, routes.rs:95-137, + 240, 380-399), SRV-09 (adapter.rs:243-256, to_openapi.rs:64-68), + GW-01/GW-03 (routes.rs:213, 257-266; error.rs:17-59), WS-02/WS-04 + (byte_adapter.rs:91-103, 135-180, 343-345; from_wss.rs:154-166, + 189-195), WS-10 (upgrade.rs:114-143), FWD-02/FWD-03/FWD-05 + (forward.rs:56-151; http_client.rs:105-173), SRV-08/COV-04 + (decoy.rs:100-136), and the vacuous test (routes.rs:1572-1583). +- Subsystem passes verified dependency-level claims against vendored + sources: axum 0.8.9 (route_layer/merge semantics, SSE, extractor body + limits), hyper 1.11 (timer gating), reqwest 0.13.4 (redirect scrub, + scheme rejection), reqwest-retry 0.9.1 (retryable strategy), + reqwest-middleware 0.5.2 (stack order), rmcp 1.8.0 (body collect, + list_all_tools, streamable-http transport, bearer application), + tokio-tungstenite/tungstenite 0.28 (max_message_size), yaml_serde + 0.10.7 (alias/depth limits), retry-policies 0.5.2 (backoff bounds), + and alkcall 0.1.1 (registry/discovery/dispatch/wire). +- Three findings were confirmed empirically with scratch harnesses + outside the repo (no repo changes): the extra-routes auth bypass + (SRV-01), the `$ref` recursion abort (OAI-01), and the SSE + chunk-boundary event loss (FWD-06). +- Coverage figures: `cargo llvm-cov --all-features` (summary + JSON + export), 2026-08-28. +- Cross-crate finding WS-12 is filed against alkcall, not this crate; + it is included because the WS path makes it trivially reachable. \ No newline at end of file