docs(tasks): post-remediation sweep — 4 follow-up tasks
Found in the sweep of completed review-001 remediation: - /publish schema validation fails open on compile error + recompiles per request (remediation-introduced, routes.rs:254-268) - OAI-06 loud-degradation unblocked and still open (HTTP_0 marker) - HY-06 ExponentialBackoff in public API + COV-02 mTLS success path both unblocked post client-config rework - HY-02/04/11 publish-prep docs gate (104 missing-docs warnings re-measured) Also flagged, not tasked here: WS-12 (alkcall demux 4 GiB discard alloc) was never actually filed in alkcall's consumer-findings-ledger — only CF-001 is there. File it when next touching alkcall. taskgraph: validate clean (42), no cycles
This commit is contained in:
@@ -0,0 +1,65 @@
|
||||
---
|
||||
id: review-001-openapi-loud-degradation
|
||||
name: Loud unsupported-OpenAPI-feature handling (OAI-06)
|
||||
status: pending
|
||||
depends_on: []
|
||||
scope: narrow
|
||||
risk: low
|
||||
impact: component
|
||||
level: implementation
|
||||
tags: [adapters, review-001, from-openapi, follow-up]
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
Review 001 finding OAI-06 — deliberately deferred during decomposition
|
||||
("revisit after the input-schema work"): unsupported OpenAPI features
|
||||
still degrade **silently**, each producing an op that misbehaves only at
|
||||
call time (verified still true post-remediation):
|
||||
|
||||
- `default`/wildcard response keys become
|
||||
`ErrorDefinition { code: "HTTP_0", http_status: None }`
|
||||
(`from_openapi.rs:build_error_schemas`, ~:238-243) — entries that never
|
||||
match a real status.
|
||||
- `trace` ops silently skipped (`openapi_spec.rs`); `servers` overrides
|
||||
ignored; parameter `style`/`explode` unsupported and silent (arrays
|
||||
serialize `"[1,2]"`); a `default`-declared SSE stream is missed by
|
||||
`detect_op_type` and would return one giant text string.
|
||||
|
||||
Since OAI-03's fix, the codebase already has the right pattern —
|
||||
`in: cookie` fails import with a clear `SchemaParse` naming the feature
|
||||
and the remediation. Extend that posture: unsupported features that
|
||||
would produce a wrong-behaving op **fail import loudly** (or degrade in
|
||||
a way that is documented, tested, and visibly warned), never silently.
|
||||
|
||||
Per-feature decision, implementer's judgment with the review's map:
|
||||
reject at import (cookie-style, preferred for anything that changes
|
||||
wire semantics: `style`/`explode` non-default forms, `servers` override
|
||||
on a non-matching base), or support (HTTP_0's real fix is mapping
|
||||
`default` → a documented catch-all code or dropping it with a warning
|
||||
— pick one and test it), or document-as-inert (`trace` skip is
|
||||
defensible if logged).
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [ ] A spec using each unsupported feature either imports with a documented, warned, tested behavior or fails import with a feature-naming error (tests per feature)
|
||||
- [ ] `HTTP_0` no longer emitted (default responses mapped or dropped loudly) — `/search` never advertises a code that can't match
|
||||
- [ ] `style`/`explode` non-default forms do not silently mis-serialize arrays
|
||||
- [ ] `cargo test` and `cargo clippy --all-targets -- -D warnings` pass
|
||||
|
||||
## References
|
||||
|
||||
- docs/reviews/001-initial-implementation-review.md (Part E, OAI-06)
|
||||
- tasks/adapters/review-001-input-schema-enforcement.md (the loud-unsupported pattern to extend)
|
||||
|
||||
## Notes
|
||||
|
||||
> Agent fills during implementation. Deferred from the original
|
||||
> decomposition for staleness reasons; unblocked once OAI-02/03/07/09
|
||||
> landed (they did). Scope guard: this is import-time fidelity, not
|
||||
> new feature support — do not implement `servers` overrides or style
|
||||
> serialization here.
|
||||
|
||||
## Summary
|
||||
|
||||
> Filled on completion.
|
||||
@@ -0,0 +1,55 @@
|
||||
---
|
||||
id: review-001-client-config-and-cert-coverage
|
||||
name: Client config API cleanup (HY-06) + mTLS/CA-bundle test coverage (COV-02)
|
||||
status: pending
|
||||
depends_on: []
|
||||
scope: narrow
|
||||
risk: low
|
||||
impact: component
|
||||
level: implementation
|
||||
tags: [client, review-001, follow-up, coverage]
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
Two deferred client-host items, grouped (same file, `src/client/http_client.rs`):
|
||||
|
||||
- **HY-06**: `HttpClientConfig.retry_policy` exposes
|
||||
`reqwest_retry::ExponentialBackoff` in the public API (`:116`) — a
|
||||
semver anchor to an upstream concrete type and an awkward construction
|
||||
surface. The post-remediation config (RetryGate, TotalRetryBudget,
|
||||
attempt counts, backoff bounds, budget deadline) is exactly why this
|
||||
should now be an owned config struct (e.g. `max_retries`,
|
||||
`max_retry_interval`, `initial_backoff`, `max_total_retry_duration`)
|
||||
from which the middleware stack builds the internal backoff. Breaks
|
||||
the public type — fine pre-publish; do it once, now, while the
|
||||
surface is small.
|
||||
- **COV-02 residue**: no test constructs a client with a working CA
|
||||
bundle or client cert — only the missing-file error paths are tested
|
||||
(`new_with_missing_ca_bundle_errors`). Post-remediation the
|
||||
redirect/retry gates hang off this client, so the TLS-config paths
|
||||
should be exercised: a test server with a self-signed cert + client
|
||||
presenting its own cert, happy path through the full middleware stack.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [ ] `HttpClientConfig` no longer exposes `ExponentialBackoff`; owned field set covers what the remediation made configurable; module docs updated
|
||||
- [ ] Existing config-construction call sites migrated (adapters' test fixtures included)
|
||||
- [ ] TLS test: client built with a CA bundle connects to a private-roots server; client-cert path exercised end-to-end (COV-02's uncovered build paths)
|
||||
- [ ] Feature matrix green (default, `--all-features`, `--no-default-features`)
|
||||
- [ ] `cargo test` and `cargo clippy --all-targets -- -D warnings` pass
|
||||
|
||||
## References
|
||||
|
||||
- docs/reviews/001-initial-implementation-review.md (Part H, HY-06; Part I, COV-02)
|
||||
- tasks/client/review-001-client-timeout-retry.md (the retry policy work this builds on)
|
||||
|
||||
## Notes
|
||||
|
||||
> Agent fills during implementation. Public-API shape change —
|
||||
> coordinate with review-001-missing-docs-sweep if running concurrently
|
||||
> (same module's docs).
|
||||
|
||||
## Summary
|
||||
|
||||
> Filled on completion.
|
||||
@@ -0,0 +1,67 @@
|
||||
---
|
||||
id: review-001-publish-schema-validation-robust
|
||||
name: Fix /publish schema validation fail-open + per-request recompilation (post-remediation)
|
||||
status: pending
|
||||
depends_on: []
|
||||
scope: narrow
|
||||
risk: high
|
||||
impact: component
|
||||
level: implementation
|
||||
tags: [gateway, review-001, follow-up]
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
Follow-up to review-001-gateway-publish-semantics (GW-01): the landed
|
||||
fix compiles the op's `publish_schema` **on every `/publish` request**
|
||||
(`src/gateway/routes.rs:254-268`) and, when compilation fails, logs a
|
||||
warning and proceeds with **no validation** — chunks flow to the handler
|
||||
unvalidated. Two remediation-introduced problems:
|
||||
|
||||
- **Fail-open on compile failure** — this reintroduces, conditionally,
|
||||
the exact transport-dependent invariant GW-01 closed: a handler that
|
||||
registered a `publish_schema` (or a remote import whose schema is
|
||||
un-compilable by jsonschema 0.46's dialect) silently receives
|
||||
arbitrary JSON over the HTTP path that the wire path would have
|
||||
aborted. A registry with one broken schema becomes an unvalidated
|
||||
ingest path with only a `tracing::warn` as the tell.
|
||||
- **Per-request compile cost** — `jsonschema::options().build()` on the
|
||||
hot path is CPU + allocation per publish call; a hostile or merely
|
||||
busy caller multiplies it.
|
||||
|
||||
Fix direction (implementer's choice on mechanism): compile **once per
|
||||
registration** — cache the compiled `Validator` (or the compile error)
|
||||
against the op in the gateway state, invalidated on re-registration —
|
||||
and make compile failure **loud and closed**: the request path surfaces
|
||||
a server-fault (500, `INTERNAL`-class with the compile error logged at
|
||||
error level), never a silent skip. Consider whether adapters can reject
|
||||
un-compilable `publish_schema` values at import/registration time so
|
||||
the failure lands before any traffic (adapter-side validation where the
|
||||
schema originates), with the gateway cache as defense in depth.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [ ] Compiled validator (or error) cached per registration; no per-request recompile (test: compile count / perf shape not asserted, but code path is registration-keyed)
|
||||
- [ ] Un-compilable `publish_schema` → request fails loudly (500/INTERNAL, error logged), chunks never flow unvalidated (test)
|
||||
- [ ] Schema re-registration (hot reload) picks up the new schema (test)
|
||||
- [ ] A `publish_schema`-registered Pub op still rejects an invalid chunk (existing GW-01 gate stays green)
|
||||
- [ ] `cargo test` and `cargo clippy --all-targets -- -D warnings` pass
|
||||
|
||||
## References
|
||||
|
||||
- docs/reviews/001-initial-implementation-review.md (Part C, GW-01)
|
||||
- tasks/gateway/review-001-gateway-publish-semantics.md (the landed fix + rationale)
|
||||
- src/gateway/routes.rs:254-268 (the fail-open)
|
||||
|
||||
## Notes
|
||||
|
||||
> Agent fills during implementation. Found in the post-remediation
|
||||
> sweep — not a Review 001 finding itself. Fail-open is the priority
|
||||
> half; caching is the efficiency half. Registration-side (adapter)
|
||||
> rejection at import is the preferred end state if alkcall's
|
||||
> registration surface allows observing `publish_schema` at
|
||||
> HandlerRegistration time.
|
||||
|
||||
## Summary
|
||||
|
||||
> Filled on completion.
|
||||
@@ -0,0 +1,64 @@
|
||||
---
|
||||
id: review-001-missing-docs-sweep
|
||||
name: missing_docs sweep + publish-prep decisions (HY-02, HY-04, HY-11)
|
||||
status: pending
|
||||
depends_on: [review-001-client-config-and-cert-coverage]
|
||||
scope: moderate
|
||||
risk: low
|
||||
impact: project
|
||||
level: implementation
|
||||
tags: [infra, review-001, docs, publish-prep]
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
The pre-crates.io documentation gate, deferred through the remediation
|
||||
cycle so the API could stabilize first (it now has):
|
||||
|
||||
- **HY-02 [major]**: ~104 missing-docs warnings under
|
||||
`RUSTDOCFLAGS="-W missing_docs" cargo doc --no-deps` (was 110 at
|
||||
review time; the re-measure is current). Worst offenders were
|
||||
`openapi_spec.rs` (28), `http_client.rs` (28), `forward.rs` (10),
|
||||
`server/adapter.rs` (10), `dispatch.rs` (7), `routes.rs` (6); plus
|
||||
`gateway/mod.rs` / `server/mod.rs` module docs. Sweep and make the
|
||||
gate pass — better, enforce it (add `-D missing_docs` to
|
||||
`RUSTDOCFLAGS` in CI or a `#![deny(missing_docs)]`-style lint config
|
||||
the repo agrees on, so it cannot regress).
|
||||
- **HY-11**: decide whether `docs/architecture/` (672 KiB) ships in the
|
||||
published package (the `exclude` list currently keeps it, omits
|
||||
`docs/reviews/`). If it ships, verify intra-doc links resolve on
|
||||
docs.rs; if not, confirm ADR references in rustdoc degrade acceptably.
|
||||
Record the decision in the task summary either way.
|
||||
- **HY-04**: the `unwrap` inside shipped `test_support`
|
||||
(`websocket/upgrade.rs:161`, behind the opt-in `test-support` feature)
|
||||
is documented-as-intentional published API with a panic inside — make
|
||||
the final call: keep (document on the public item) or restructure to
|
||||
return `Result`.
|
||||
|
||||
Depends on the client task because `http_client.rs` is 28 of the
|
||||
warnings and that task reshapes the config surface it would document.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [ ] `cargo doc` with `-W missing_docs` (as deny) exits clean — 0 warnings
|
||||
- [ ] Enforcement landed (CI rustdocflags or lint config) so it stays clean
|
||||
- [ ] HY-11 decision recorded; `cargo publish --dry-run --allow-dirty` package contents match the decision
|
||||
- [ ] HY-04 resolved (documented or restructured)
|
||||
- [ ] `cargo test`, `cargo clippy --all-targets -- -D warnings`, `cargo fmt --check` pass
|
||||
|
||||
## References
|
||||
|
||||
- docs/reviews/001-initial-implementation-review.md (Part H, HY-02, HY-04, HY-11)
|
||||
|
||||
## Notes
|
||||
|
||||
> Agent fills during implementation. The `cargo tree -d` and HY-10
|
||||
> link items: HY-10's ADR-051 placeholder link in
|
||||
> `src/adapters/from_openapi.rs:12` still resolves to a
|
||||
> `https://docs.rs/alkhttp` placeholder — fix remaining placeholder/
|
||||
> relative links opportunistically in this sweep and mark HY-10 fully
|
||||
> resolved.
|
||||
|
||||
## Summary
|
||||
|
||||
> Filled on completion.
|
||||
Reference in New Issue
Block a user