Outbound HTTP design spec#275
Draft
aram356 wants to merge 250 commits into
Draft
Conversation
Turns on `pedantic` (warn) and `restriction` (deny) workspace-wide and adds
`[lints] workspace = true` to every crate so the policy actually applies.
Captures a baseline allow-list in `Cargo.toml`, organized by category
(Documentation, Style/formatting, Defensive coding, API design, Imports/paths,
Output/diagnostics, Tests, Attributes) with per-lint counts and rationales —
each entry is a TODO unless explicitly marked intentional.
Defensive-coding pass:
- New `clippy.toml` with `allow-{unwrap,expect,panic,indexing-slicing}-in-tests`
so test code keeps its conventional idioms; production code is denied.
- Production unwraps factored out: `current_dir()`/`init_logger()` now
propagate via `?`; `writeln!` to a `String` rewritten as `push_str(&format!)`
so there's no `Result` to discard; bundled-template registration and other
genuine compile-time invariants use `.expect("...")` as documented assertions.
- Other small wins: `inefficient_to_string` fixed, `match_same_arms` collapsed,
`manual_assert` swapped, `cast_lossless`+truncation replaced with bound-checked
`u16::try_from` in adapter-axum CLI, `unreachable!()` in `#[action]` macro
replaced with a proper `syn::Error::compile_error`.
Lints kept allowed in the workspace are annotated with `(intentional)` where
they conflict with idiomatic Rust (`implicit_return`, `question_mark_used`,
`pattern_type_mismatch`, `default_numeric_fallback`, `arithmetic_side_effects`,
`as_conversions`, `string_slice`) or have no per-test config option
(`assertions_on_result_states`).
`cargo clippy --workspace --all-targets --all-features -- -D warnings`,
`cargo fmt`, and `cargo test --workspace --all-targets` all pass.
Drives the API-design lint group from 18 allows down to 8 (kept as intentional
with rationale comments in `Cargo.toml`).
Factored out:
- `return_self_not_must_use` (18): added `#[must_use]` to all `RouterBuilder`
builder methods. Catches "I forgot to call `.build()`" bugs.
- `impl_trait_in_params` (26): converted `fn f(x: impl Into<String>)` →
explicit generics on `EdgeError::*`, `ConfigStoreError::*`, `RouteInfo::new`,
`InMemorySecretStore::new`, `AxumConfigStore::{new,from_env,from_lookup}`.
Makes turbofish callable.
- `rc_buffer` (4): `Arc<Vec<RouteInfo>>` → `Arc<[RouteInfo]>` in `RouterInner`
and the builder. Saves an indirection.
- `unnecessary_wraps` (4): `build_fastly_request` and `convert_response` no
longer wrap an always-Ok value in `Result`. Cleaner call sites.
- `mutex_atomic` (1): `Arc<Mutex<bool>>` → `Arc<AtomicBool>` in the
`middleware_fn` test.
- `ref_patterns` (11): `if let Some(ref x) = ...` → `if let Some(x) = &...`
across env-override `Drop` impls, router builder, response builder, body
matchers.
- `wildcard_enum_match_arm` (7): `args.rs` tests now use `let-else` instead of
catch-all wildcard match arms; `EdgeError::source` now lists each non-Internal
variant explicitly; `cli/build.rs` switched to `if let Value::Table(_) = ...`;
the one site that genuinely matches an external enum (`fastly::config_store::
LookupError`) keeps a localized `#[allow(..., reason = "external enum")]`.
- `clone_on_ref_ptr` (1): `store.clone()` → `Arc::clone(&store)` in the axum
service test (with explicit `Arc<dyn KvStore>` annotation so `Arc::clone`
picks the right type).
- `renamed_function_params` (4): renamed `request: Request` → `req: Request`
in `Service::call` impls to match the trait signature.
- `same_name_method` (2): `EdgeError::source` deliberately shadows
`std::error::Error::source` (typed `&AnyError` vs trait-object `&dyn Error`).
Documented at the call site with a `#[allow(..., reason = "...")]`.
Kept allowed (with `(intentional: ...)` comments in `Cargo.toml`):
- `exhaustive_structs` (108) and `exhaustive_enums` (18): blanket
`#[non_exhaustive]` would break user pattern matching and field-syntax
construction. Apply per-type only when genuinely planned.
- `must_use_candidate` (117): most flagged sites are getters returning
`&str`/`&Path` — ignoring is impossible, the lint adds noise.
- `missing_trait_methods` (20): relying on default trait methods is fine.
- `needless_pass_by_value` (16): most flagged sites are deliberate ownership
transfers — error transformers, proc-macro signatures, builders.
- `field_scoped_visibility_modifiers`, `partial_pub_fields`,
`trivially_copy_pass_by_ref`: deliberate API design choices.
Final clippy + workspace tests pass.
Following pushback that the prior passes were papering over lints rather
than addressing them, this commit revisits each lint that was previously
allowed with hand-wavy reasoning and either (a) factors it out for real,
(b) applies it selectively where the fix matters, or (c) replaces the
rationale with a per-site audit finding.
Real fixes:
- `Body::as_bytes` and `Body::into_bytes` no longer panic on streaming
bodies — they return `Option`. This eliminates two production panic
sites the previous pass left as `panic = "allow"`. The internal
`into_bytes_bounded` site is correctly gated by `is_stream()`; all
other callers are tests that *intentionally* assert the body is
buffered, now with `.expect("buffered")`.
- `assertions_on_result_states` is no longer allowed. All 13 sites
converted from `assert!(r.is_ok())` / `assert!(r.is_err())` to
`r.expect("...")` / `r.expect_err("...")` — these print the value or
error on failure instead of just `assertion failed: false`.
- `#[non_exhaustive]` applied to all 4 error enums (`EdgeError`,
`KvError`, `SecretError`, `ConfigStoreError`) and the 3 manifest
enums (`HttpMethod`, `BodyMode`, `LogLevel`) — this is the idiomatic
Rust pattern for error/config enums (see `std::io::ErrorKind`,
`serde::de::Error`). Also applied to 19 deserialize-only manifest
structs (`Manifest*`, `ResolvedEnvironment*`-where-not-constructed-
externally).
- `needless_pass_by_value` real fix in `run_app_with_stores`:
`FastlyLogging` and `StoreRequirements` are now passed by reference
since the function only reads from them.
Lints kept allowed but with audited per-site rationales (replacing the
previous one-line hand-waves):
- `pattern_type_mismatch`: every flagged site uses Rust 2018
match-ergonomics. The "fix" reverts to manual `ref` patterns or
explicit `&Variant(...)` arms, both worse.
- `arithmetic_side_effects`: every site is bounded by domain invariants
(TTL+now, path component counts, byte offsets after `len()` checks).
- `as_conversions`: dominated by trait-object coercions (`Arc::new(x)
as BoxMiddleware`) which cannot be expressed as `From`/`Into` in
stable Rust.
- `string_slice`: every flagged site indexes ASCII-only data (env var
names, header names, `matchit` path components).
- `expect_used`: 62 production sites audited — bundled-template
registration, AsyncRead-contract slice access, lock-poisoning
unrecoverable, build-script panics. None benefit from `?`
propagation.
- `panic`: route-registration `unwrap_or_else(|err| panic!(...))` and
proc-macro expansion failures. Both build/setup-time programmer
errors, not runtime conditions.
- `cast_possible_truncation` / `cast_sign_loss`: narrowing/sign casts
always preceded by range checks.
- `exhaustive_structs` / `exhaustive_enums`: applied selectively above;
remaining sites are tuple-struct extractors users *destructure*,
unit structs, externally-constructed scaffold blueprints, request-
context types used in integration tests, and small enums (`Body`,
`AdapterAction`) where adding `#[non_exhaustive]` would force 12+
adapter sites to add never-firing wildcard arms.
Workspace clippy + tests still pass with `-D warnings`.
Removes 22 mechanical-fix allow entries from `Cargo.toml` after fixing the
underlying call sites:
Auto-fixed (`cargo clippy --fix` + manual cleanup):
- `uninlined_format_args` (180), `redundant_closure_for_method_calls` (25),
`map_unwrap_or` (29), `explicit_iter_loop` (14),
`unseparated_literal_suffix` (24, separated form chosen),
`implicit_clone` (2), `pathbuf_init_then_push` (3), `string_add` (3),
`unreadable_literal` (4), `manual_let_else` (2), `else_if_without_else`
(2 — the Fastly-vs-other-adapter logging branch refactored to a
pre-computed `Option<endpoint>`), `return_and_then` (2), `ip_constant`
(2), `manual_string_new` (1), `redundant_type_annotations` (1),
`needless_raw_strings` (1), `needless_raw_string_hashes` (1),
`elidable_lifetime_names` (2), `redundant_test_prefix` (1),
`if_then_some_else_none` (6), `deref_by_slicing` (5), `shadow_same` (4),
`match_wildcard_for_single_variants` (5), `pub_with_shorthand` (30),
`decimal_literal_representation` (1).
Real fixes (manual):
- `key_value_store.rs`: replaced bare scoping blocks `{ ...?; }` with
explicit `drop(table)` so neither `semicolon_inside_block` nor
`semicolon_outside_block` fires (the lint pair is mutually exclusive
and one always fires). Same treatment for `decompress.rs` and
`proxy.rs` brotli-test compressor scopes.
- `middleware.rs`: collapsed the `Mutex` lock+await pattern into a
single `self.log.lock().unwrap().push(...)` statement so the lock
guard drops immediately (was previously triggering
`await_holding_lock` after I removed the scoping block).
- `dev_server.rs`: `let service = service` (shadow_same) refactored
into a `let service = { mut service = ...; ...; service }` block
expression that yields the configured value.
- `response.rs`: dropped redundant `let stream = stream` shadow.
- `request.rs`: renamed `test_is_json_content_type` →
`json_content_type_detection` (the redundant `test_` prefix).
- `proxy.rs` test panics: `_ => panic!(...)` → `Body::Stream(_) =>
panic!(...)` so the match stays exhaustive when `Body` grows.
- `cli.rs`: `0xFFFF` instead of `65535` for the u16-MAX boundary.
- `dev_server.rs::stable_store_name_hash`: split FNV-1a magic numbers
with `_` separators.
The Style section in `Cargo.toml` is rewritten as a tight allow-list
(no narrative, no historical commit log inside the manifest). Each
remaining entry has a one-line rationale grouped by category:
- Idiomatic Rust (8 lints): `implicit_return`, `min_ident_chars`,
`single_call_fn`, `single_char_lifetime_names`, `pub_use`,
`str_to_string`, `question_mark_used` (was duplicated; consolidated
in Defensive section).
- Mutually-exclusive pairs we picked one side of: `separated_literal_suffix`,
`pub_with_shorthand`.
- Held-by-choice (5 lints): `format_push_string`, `shadow_reuse`,
`shadow_unrelated`, `similar_names`, `non_ascii_literal`,
`too_many_lines`, `arbitrary_source_item_ordering`,
`module_name_repetitions`.
Allow-list went from ~80 entries to 57 across all categories.
`cargo clippy --workspace --all-targets --all-features -- -D warnings`
and `cargo test --workspace --all-targets` both pass.
`#[action]` requires the user-written fn to be `async fn` because the generated outer fn `.await`s it. When a handler body has no awaits of its own, `clippy::unused_async` fires on the user's source — but the user has no choice; the macro forces `async`. Inject the allow into the inner fn's attribute list inside the macro expansion so handler authors don't have to know about the lint.
Imports/paths track:
- `non_std_lazy_statics` (6 sites): `once_cell::Lazy` → `std::sync::LazyLock`
in `crates/edgezero-adapter/src/{registry,scaffold}.rs`. Drops `once_cell`
from `crates/edgezero-adapter/Cargo.toml`. (Workspace dep stays — example
app still uses it.)
- `unused_trait_names` (37 sites): `use Foo;` → `use Foo as _;` for traits
imported only for their methods (`StreamExt`, `Write`, `Read`, `Hooks`,
`IntoHandler`, `Spanned`, etc.) across both library and proc-macro crates.
- `iter_over_hash_type` (1 site): the only flagged production iteration is
in `RouterInner::dispatch` (collecting allowed methods for a 405 response).
Refactored from a `for ... { allowed.insert(...) }` loop into
`.iter().filter().map().collect::<HashSet<_>>()`. The result is a `HashSet`
whose order doesn't matter (`EdgeError::method_not_allowed` sorts on render).
Attributes track:
- `allow_attributes` (3 sites): `#[allow(...)]` → `#[expect(..., reason)]` on
the genuine deliberate-shadowing/wildcard-match-arm sites in
`error.rs::EdgeError::source` and `config_store.rs::map_lookup_error`. The
CLI build script (`build.rs`) now emits `#[expect(unused_imports, reason)]`
on every generated `pub(crate) use` re-export.
- `allow_attributes_without_reason` (5 sites): every existing `#[allow(...)]`
now has a `, reason = "..."` and (where stable-`expect` applies) is migrated
to `#[expect(...)]`. Sites: `cli_support.rs` and `decompress.rs` top-of-file
`#![expect(dead_code, ...)]`; the four test-only `Deserialize` field structs
in `context.rs` and `params.rs`; the macro's `manifest_definitions` shim;
the two fastly `deprecated` re-exports.
Also kept allowed (real audits in `Cargo.toml` rationales):
- `absolute_paths` (200+ sites): one-shot `std::env::var()` / `std::fmt::Display`
uses; adding `use` statements wouldn't improve readability for single-use.
- `std_instead_of_alloc` / `std_instead_of_core`: not targeting `no_std`.
- `tests_outside_test_module`: lint matches plain `#[cfg(test)] mod tests`
only — doesn't recognize `#[cfg(all(test, feature = "..."))]` or
integration-test files in `tests/`.
- `print_stderr` / `print_stdout`: kept in CLI top-level error reporters and
status output (`[edgezero] creating project at ...`).
Allow-list now at 51 entries.
…c / doc_markdown / missing_fields_in_debug Adds public-API docs across every flagged site: - `missing_panics_doc` (28 sites): added `# Panics` sections describing each panic condition. Most are documented invariants (lock poisoning, AsyncRead-contract slice access, builder pre-validated headers); a few are caller-controlled (`enable_route_listing_at` asserts on path shape, `RouterBuilder::build` panics on duplicate route, `load_from_str` panics on invalid embedded TOML — the docs note safer alternatives). - `missing_errors_doc` (62 unique pub fns, 124 lints with re-exports): added `# Errors` sections describing the concrete error variants returned. Dispatched via batch script with per-fn descriptions covering every site (KV / secret / config-store / manifest / proxy / extractor / body / responder / middleware / adapter dispatch APIs). - `missing_fields_in_debug` (2 unique sites — 4 with re-exports): `ProxyRequest`/`ProxyResponse` `Debug` impls now use `finish_non_exhaustive()` to acknowledge the deliberately-skipped `body` and `extensions` fields. - `doc_markdown` (17 sites): backticked `EdgeZero`, `SystemTime`, `Axum`, `SecretStore`, etc. in doc comments. Lints kept allowed (with rationale comments in `Cargo.toml`): - `missing_docs_in_private_items` (275 sites): private docs aren't load-bearing for users — industry-standard "kept allowed". - `missing_inline_in_public_items`: `#[inline]` is a perf hint; rustc/LLVM make better decisions than blanket-marking every cross-crate public item. Allow-list: 51 → 47 entries.
…t_stdout allows
The CLI binary now initializes a `simple_logger` with no timestamps and no
level prefixes (so the user-facing UX is unchanged: `[edgezero] creating
project at ...` still prints exactly that), and all `println!` /
`eprintln!` sites are converted to `log::info!` / `log::error!` /
`log::warn!`.
Sites converted (24 total):
- `crates/edgezero-cli/src/main.rs`: top-level error reporters (`new`,
`build`, `deploy`, `serve`, `dev`) + status output for store-binding
warnings.
- `crates/edgezero-cli/src/generator.rs`: 9 status messages and 2 git
warnings now go through the logger.
- `crates/edgezero-cli/src/dev_server.rs`, `adapter.rs`: dev manifest /
command-failure reporting.
- `crates/edgezero-adapter-{axum,cloudflare,fastly,spin}/src/cli.rs`:
one build-artifact-path message each.
Allow-list: 47 → 45 entries (`print_stderr` + `print_stdout` removed).
Real renames + restructuring (no inline allow attrs):
- `non_ascii_literal` (3 sites): replaced the Japanese KV-key test literal
with `\u{...}` escapes (same runtime bytes, ASCII source) instead of
`#[expect]`-ing the lint. Replaced `→` arrow in a CLI test message with
`->`.
- `similar_names` (2 sites): renamed `decoded` → `output` in
`crates/edgezero-adapter-spin/src/decompress.rs` to break the
`decoded`/`decoder` prefix-share that the lint flags.
- `too_many_lines` (1 site): split `collect_adapter_data` in
`crates/edgezero-cli/src/generator.rs` into three helpers
(`blueprint_data_entries`, `render_manifest_section`,
`append_readme_entries`).
- `shadow_unrelated` (~14 sites): renamed every flagged inner binding
to be specific to its purpose:
- `serve_with_stores`: `let router = Router::new()...` →
`axum_router`; `let server = server.with_graceful_shutdown(...)` →
`graceful_server`; `let shutdown = ...` → `shutdown_signal`.
- `store_name_slug`: `Some(ch)` → `Some(lower_ch)` (was shadowing
outer `ch`).
- dev_server tests: `let url = ...` reused per-step → `write_url`,
`read_url`, `check_url`, `delete_url`, `save_url`, `load_url`;
`let resp = ...` → `write_response`/`read_response`/`save_resp`/
`load_resp`/`exists_before`/`exists_after`.
- `axum::key_value_store::get_bytes`: inner write-txn `table` →
`write_table`, `entry` → `fresh_entry`.
- `list_keys_page` cursor match: inner `Some(cursor)` → `Some(scan_from)`.
- `data_persists_across_reopens` test: second `let store = ...` →
`reopened`.
- `axum::response::into_axum_response` error path: `body` →
`error_body`, `response` → `error_response`. Test: `stream` →
`body_stream`.
- `fastly::key_value_store::list_keys_page`: inner `cursor` →
`next_cursor`.
- `fastly::proxy` test: collapsed two pairs of `body`/`collected` reuse
into named bindings (`plain_body`, `gzip_body`).
- `spin::decompress` test: `let result = ...` reused per-encoding →
`none_encoding`, `identity_encoding`.
- `core::body::from_stream_maps_errors` test: `stream` →
`source`/`chunks`.
- `core::key_value_store` tests: `let val = ...` reused → `after_first`/
`after_second`/`int_val`/`str_val`/`single_dot_err`/`double_dot_err`.
- `axum::cli::read_axum_project`: `Some(value)` → `Some(port_value)`
(was shadowing outer `value` from `toml::from_str`).
Allow-list: 45 → 41 entries.
…quest path
Real fixes (not just docs) for every production-code .expect() that could
fire under upstream contract change or misconfigured input:
- `IntoResponse::into_response` now returns `Result<Response, EdgeError>`
workspace-wide (breaking change). Cascades through `Responder`,
`EdgeError::into_response`, `RouterService::oneshot`, the handler future
in `core/handler.rs`, and the route-listing builder.
- `ProxyResponse::into_response` and `core::response::response_with_body`
now return `Result<Response, EdgeError>` and propagate `http::Builder`
failures via `map_err(EdgeError::internal)?` instead of `.expect()`.
- `core::body::Body::into_bytes_bounded` rewritten as a `match self {
Once | Stream }` so the unreachable `is_stream()`-guarded `.expect()`
pair is gone — the compiler proves exhaustiveness.
- `core/compression.rs` decoder slice access now propagates as
`io::Error::other(...)` instead of `.expect("AsyncRead contract")`,
so a malicious or buggy upstream stream fails the request rather than
crashing the worker.
- `axum/response.rs::into_axum_response` error path no longer uses
`Response::builder().expect(...)`; constructs the 500 response
directly via `Response::new` + `status_mut` + `headers_mut().insert`,
every step infallible by `http`-crate contract.
- `axum/proxy.rs` replaced `Default` (which panicked on TLS init) with
fallible `AxumProxyClient::try_new() -> Result<_, reqwest::Error>`.
Production caller in `request.rs::into_core_request` propagates as a
`String` error (matches the fn's existing return type).
- `fastly/logger.rs::init_logger` now returns
`Result<(), InitLoggerError>` (a typed enum wrapping the underlying
build error and `log::SetLoggerError`) instead of `.expect("non-empty
Fastly logger endpoint")`. `lib.rs::init_logger` re-exports the wider
return type.
- `cli/generator.rs::render_templates` propagates the previously-
`.expect("adapter context dir has a file name")` invariant as
`io::Error::other` since the surrounding fn already returns
`io::Result<()>`.
`axum/service.rs::call` (the tower `Service` impl) bridges the new
`Result<Response, EdgeError>` from `RouterService::oneshot` into a
`Response<AxumBody>` by mapping the error to a hard-coded 500 with a
plain-text body — `Service::call` returns `Result<Response, Infallible>`
so we cannot propagate further up the stack here.
`adapter-fastly` adds `thiserror` as a direct dependency for
`InitLoggerError`. All 557 workspace tests still pass.
Replaces the previous \`std::io::Result<()>\` / \`io::Error::other(format!(...))\`
shape across the \`edgezero new\` code path with two domain-specific error
types:
- \`crate::scaffold::ScaffoldError\` (variants \`Io { path, source }\` and
\`Render { name, message }\`) wraps every Handlebars failure and every
filesystem op inside template rendering with the offending path/template
name attached.
- \`crate::generator::GeneratorError\` (variants \`OutputDirExists\`,
\`AdapterDirMissingFileName\`, \`Io { path, source }\`, and
\`Scaffold(#[from] ScaffoldError)\`) replaces the workspace-construction
io::Error stringification.
\`generate_new\`, \`ProjectLayout::new\`, \`collect_adapter_data\`, and
\`render_templates\` all return \`Result<_, GeneratorError>\`.
\`adapter-cli\` and \`scaffold\` now depend on \`thiserror\` directly. All
557 workspace tests still pass.
The `IntoResponse::into_response` change in 1506738 turned the trait into `-> Result<Response, EdgeError>` workspace-wide. The demo app (`examples/app-demo/`) is excluded from the main `Cargo.toml` workspace, so it didn't get rebuilt by the workspace clippy/test gate and silently broke. This propagates the same fix to the demo: - Every `block_on(handler(ctx)).expect("handler ok").into_response()` in `crates/app-demo-core/src/handlers.rs` test code now appends `.expect("response")` to unwrap the response result. - Every `into_body().into_bytes()` test path now appends `.expect("buffered")` since `Body::into_bytes()` returns `Option<Bytes>` (changed in the defensive-coding pass). `cd examples/app-demo && cargo test --workspace --all-targets` passes all 21 demo handler tests; `cargo clippy --workspace -- -D warnings` also clean.
Inherit pedantic+restriction lints in the demo workspace and each demo crate. Fix the lints that flagged real issues in the demo handlers (`as _` trait imports, inlined format args, fast-path `to_string`, renamed shadowed bindings, separated literal suffix). The demo's allow-list is intentionally narrower than the library's — only entries the demo actually trips. New allows can be added lazily as future failures surface.
Add a clippy.toml mirroring the parent (allow expect/unwrap/panic/ indexing-slicing in tests). Then refactor away the workspace allows that were genuine wins: - shadow_reuse: rename `chunk` and `cursor` shadows - absolute_paths: import std::env, std::time::Duration, std::process, and use already-imported Arc instead of std::sync::Arc - default_numeric_fallback: add type suffixes (1_u64, 0_i32..3_i32, 1_i64) - pattern_type_mismatch: implicitly fixed by str_to_owned changes - missing_trait_methods: implement KvStore::exists on the test MockKv - expect_used in production code: stream() now propagates the response builder error via EdgeError::internal The remaining allow-list keeps only entries the demo actually trips that match main's philosophical stance — std (not core/alloc) for binaries, idiomatic `?` over match, terse closure idents, and the single exhaustive_structs site that comes from the `app!` macro.
- str_to_string (21 sites): `.to_string()` → `.to_owned()` on `&str` - arithmetic_side_effects: counter `n + 1` → `n.wrapping_add(1)` - min_ident_chars + pattern_type_mismatch: rename closure destructures `|(k, v)|` → `|&(name, value)|`/`|&(key, value)|` - pub_with_shorthand + field_scoped_visibility_modifiers: drop `pub(crate)` shorthand on the demo's DTOs and handlers — the `mod handlers;` declaration is already private, so plain `pub` is crate-private at the boundary - print_stderr: axum main returns `anyhow::Result<()>` and lets the Termination impl render errors; fastly/cloudflare host stubs keep `eprintln!` behind a localized `#[expect]` with reason since they only run on the wrong target Workspace allow-list now keeps only the entries that match main's philosophical stance (idiomatic `?`, `pub` shorthand handled per-call site, etc.) plus the single `exhaustive_structs` site from the `app!` macro.
Drop the `arbitrary_source_item_ordering` allow in favor of the canonical clippy-restriction layout: - Top of `handlers.rs`: consts (alphabetical), then structs (alphabetical: ConfigParams, EchoBody, EchoParams, NoteIdPath, ProxyPath), then handler fns - Test mod: uses, then structs (alphabetical), then impls grouped with their self-types, then helper + test fns interleaved in alphabetical order - `impl KvStore for MockKv` methods alphabetical (delete, exists, get_bytes, list_keys_page, put_bytes, put_bytes_with_ttl) - Hoisted the late `use edgezero_core::secret_store::...` up to the test mod's use block No behavior changes — pure reordering. Demo workspace allow-list drops to 8 entries.
The `edgezero new` generator now scaffolds the same lint policy EdgeZero itself uses: - Root `Cargo.toml` carries `[workspace.lints.clippy]` (pedantic warn + restriction deny) with the same demo-tested allow-list - Root `clippy.toml` exempts tests from `unwrap`/`expect`/`panic`/ indexing-slicing restriction lints - Each generated crate's Cargo.toml inherits via `[lints] workspace = true` Generated projects are clippy-clean against the strict gate out of the box.
Both adapters were calling `from_core_response` directly on the router's return value, but `oneshot` now yields `Result<Response, EdgeError>` since the response builder errors propagate through the router. Extract the response with `?` first so the wasm32 builds (`--target wasm32-unknown-unknown` for cloudflare, `--target wasm32-wasip1` for spin) compile again.
… per-site Real fixes (allows now justified by audit, not laziness): - build.rs returns `Result<(), Box<dyn Error>>` instead of expect-panicking - adapter registry / blueprint registry recover from poisoned RwLocks via `unwrap_or_else(PoisonError::into_inner)` rather than expect-panicking - ManifestLoader gains `try_load_from_str` returning `io::Result`; adapter `run_app` paths propagate via `?`. The non-fallible `load_from_str` keeps its panic-on-bad-input contract for compile-time-embedded manifests, with a documented per-fn `#[expect(clippy::panic, reason = ...)]` - `expand_app` macro emits `compile_error!()` instead of panicking on bad `edgezero.toml` (rustc surfaces a clean build error) - `parse_handler_path` keeps a panic with a clear reason — proc-macro expansion errors *are* build failures - `partial_pub_fields` on `Manifest`: privatized `root` and `logging_resolved`, kept the deserialized fields `pub` for the public API. Localized `#[expect]` documents the deliberate split - `must_use_candidate` fixed on cli_support helpers via `#[must_use]` - `missing_inline` fixed on adapter/scaffold registry functions - `pub_use`, `format_push_string`, `arithmetic_side_effects`, `default_numeric_fallback`, `pattern_type_mismatch`, `min_ident_chars`, `str_to_string`, `absolute_paths`, `module_name_repetitions`, `shadow_reuse`: all kept as workspace allows but with concise rationales replacing the prior verbose audit notes Each remaining workspace allow now has a one-line reason. The list is shorter than before but explicitly accepts the lints whose "fix" would universally make the code worse (match-ergonomics destructures, std-only binary entrypoints, idiomatic `?`/return).
…space-wide 54 sites across 23 files. Fixed places where my bulk replace had wrongly converted Display::to_string() calls (anyhow::Error, io::Error, i32 etc.) back to .to_string(). The lint allow is dropped from the workspace.
23 sites across extractor.rs, key_value_store.rs, middleware.rs, proxy.rs, adapter-axum dev_server/key_value_store, adapter-spin decompress. Validator length(min=N) gets _u64; range(min=N, max=N) gets matching type suffix; loop-bound and assertion literals get explicit i32.
Core crate: replaced 60+ `std::collections::HashMap`, `std::sync::Arc`, `std::ops::Deref/DerefMut`, `crate::error::EdgeError`, `futures::executor::block_on`, `std::task::*`, `std::string::String::*` absolute paths with explicit `use` statements. Axum proxy.rs: imported the various `axum::http::*` and `axum::routing::*` types used in test functions. The lint stays allowed at the workspace level for adapter test modules where one-shot uses of framework types like `axum::http::HeaderMap` and `fastly::kv_store::KVStore` are clearer inline.
Real fixes (workspace allows dropped, code refactored): - AdapterAction marked #[non_exhaustive] with wildcard arms in adapter cli match sites — drops a workspace exhaustive_enums concession - Adapter crate exposes `pub mod registry` instead of pub-using items at the crate root — drops the workspace pub_use concession - expand_action_impl made private (no longer pub(crate)) — drops the workspace pub_with_shorthand concession on this site - ManifestLoader, Manifest, ManifestApp/HttpTrigger/Environment/Binding/ ResolvedEnvironment*, ManifestAdapterBuild/Commands, ManifestConfigStoreConfig, ManifestLoggingConfig, ResolvedLoggingConfig, ManifestKvConfig, ManifestSecretsConfig, HttpMethod, LogLevel — all reordered to match canonical clippy item ordering (consts first, then structs, impls, fns; alphabetical within each group) - Manifest impl methods sorted alphabetically; Manifest fields sorted - match-ergonomics destructures rewritten as let-else for clarity - HttpMethod gained Copy; LogLevel/HttpMethod take `self` (drops trivially_copy_pass_by_ref) - partial_pub_fields fixed via consistent pub on Stores in fastly request - needless_pass_by_value: run_app_with_config / run_app_with_logging take `&FastlyLogging`; map_edge_error / map_lookup_error take by ref; build_fastly_request takes `&HeaderMap`; generate_new takes `&NewArgs` - expect_used localized on register_templates with rationale - ManifestLoader::load_from_str / parse_handler_path keep panic-on-bad- build-input contract documented per-fn - Router: route-listing duplicate-path panic + add_route panic both documented per-fn (build-time programmer error) - spin contract test uses #[allow] for expect/tests-outside per file - separate manifest_definitions.rs in macros crate (drops mod-after-use) Workspace allows that survived (most match audited rationales): implicit_return, question_mark_used, single_call_fn, separated_literal_suffix, pub_with_shorthand (rustfmt-enforced), pub_use, min_ident_chars, single_char_lifetime_names, shadow_reuse, module_name_repetitions, format_push_string, pattern_type_mismatch, arithmetic_side_effects, float_arithmetic, as_conversions, exhaustive_structs, exhaustive_enums, missing_trait_methods, absolute_paths, std_instead_of_alloc/core, missing_inline_in_public_items, tests_outside_test_module, arbitrary_source_item_ordering (core-crate files outside manifest.rs). Tests pass, strict clippy clean across workspace + demo.
Override KvStore::exists in 4 production impls (axum/fastly/cloudflare + NoopKvStore) and the in-test MockStore. Override configure/name/ config_store/build_app in the two Hooks test impls. Update the #[app] macro to emit configure, build_app, and a None-returning config_store when [stores.config] is absent so generated user apps still pass clippy. Add explicit clone_from to RouteEntry's Clone impl.
Delete config_store, key_value_store, and secret_store crate-root
re-exports — items remain reachable via the `pub mod` paths. Update the
two short-path callers (axum service.rs / secret_store.rs) to use full
module paths. Keep `pub use edgezero_macros::{action, app}` and the
`http` facade re-exports — these are the only surviving sites and the
lint is module-scoped so it cannot be silenced per-item. Workspace
allow rationale updated to point to those two patterns.
The previous comment framed `push_str(&format!(...))` as a stylistic preference. It is actually the only call-site form that satisfies the full restriction-deny gate: `write!(s, ...)` returns a `Result` which trips `let_underscore_must_use` under `let _ =`, `unwrap_used` under `.unwrap()`, and `expect_used` under `.expect()`.
Switch generator.rs from `push_str(&format!(...))` to `writeln!(...)?` which writes directly into the buffer (no temp String allocation) and propagates `std::fmt::Error` rather than silencing it. Add `GeneratorError::Format(#[from] std::fmt::Error)` and bubble the result through `render_manifest_section` and `append_readme_entries`. Drop the workspace allow.
Rename 'a → 'mw on Next, 'a → 'route on RouteMatch, 'a → 'manifest on manifest_command, and 'a → 'blueprint on AdapterContext. Drop the workspace allow.
Eliminate let-rebinding shadows across core, fastly, axum, and cli
crates. The recurring patterns:
- `while let Some(chunk) = stream.next().await { let chunk = chunk?; }`
→ rename outer to `result`, keep inner `chunk`
- `if let Some(cursor) = cursor.filter(...)` → rename outer/inner to
distinct names
- `let path = path.into()` (Into-paramter idiom) → rename to
destination-specific name
- closure params shadowing outer captures → rename closure param
All renames preserve semantics; tests + workspace clippy + wasm
target checks all pass.
Split `#[cfg(all(test, feature = "..."))]` on test modules into two separate cfg attributes (`#[cfg(test)] #[cfg(feature = "...")]`) which the lint recognizes correctly. Affects edgezero-adapter-fastly lib.rs and edgezero-cli main.rs.
Adds the EdgeZero outbound HTTP design spec under docs/superpowers/specs/2026-05-21-outbound-http-design.md. Targets the PR #269 (feature/extensible-cli) baseline. The spec covers six requirements: - portable OutboundHttpClient trait with single send + concurrent send_all on every adapter (Axum, Cloudflare, Fastly, Spin) - per-request and shared deadline / timeout primitives with a documented dispatch budget and bounded-cooperative semantics on Fastly - bounded buffering with explicit persistent vs transient memory accounting and pre-append cap checks - manifest-driven [capabilities] declaration (nine capabilities total) with pre-dispatch enforcement gates at five CLI entry points - adapter contract test plan in three tiers (core mock, per-adapter translation, runtime) - four canonical URI accessors (backend_target, host_authority, sni_hostname, cert_host) so adapters share one canonical host/port/SNI/cert split Revised through 49 review rounds; non-normative resolution journal lives in Appendices A through AX.
aram356
changed the title
aram356
marked this pull request as draft
June 8, 2026 06:55
Removes references to the originally-named driving consumer and the specific external protocol used as motivation: - midbid → "the driving pattern" / generic - Prebid-style → "fan-out-style" - OpenRTB → "the external batch protocol" - bidder → "target" - auction → "fan-out batch" - tmax → "batch deadline" The technical motivation (N concurrent outbound requests under a shared wall-clock deadline, results harvested in input order, small response bodies, homogeneous-budget common case) is preserved; only the named consumer and its protocol are scrubbed so the spec reads as a portable substrate rather than a single-consumer design. Section §3.3.2 retitled "Mapping an external batch deadline to EdgeZero deadlines"; status header gains a "Driving pattern" line in place of the old "Driving consumer" pointer.
Spec previously claimed Fastly behaviour that did not match the actual SDK / public API. This commit corrects the four normative claims, adds an app-facing consuming body accessor, and records the corrections in Appendix AY. Findings addressed: - lazy-streamed-response-passthrough downgraded Native -> BestEffort on Fastly. `Response::with_streaming_body` does not exist; `Response::stream_to_client()` is the actual API and is documented as incompatible with `#[fastly::main]`. Default scaffold falls back to buffered passthrough; lazy passthrough requires the non-main entry-point template tracked in new section 8 risk 12. - NameInUse semantics rewritten. Fastly's session-uniqueness rule is unconditional; the previous "identical name + identical properties is a re-registration that returns Ok" carve-out was false. SDK's `Backend::from_str(name)` returns a handle only and exposes no registered properties, so a NameInUse on a name not in this adapter's collision map is now an explicit fail-closed internal error rather than a silent property-trust fallback. - between_bytes_timeout is receive-side only per Fastly's Backend API docs. The previous claim that it bounded guest-to-origin writes is removed; the streamed-upload host-write phase is downgraded to BestEffort with the cooperative inter-chunk check as the only adapter-side bound. - Streamed-upload response overshoot tightened from per-chunk accumulator to closed-form bound: first_byte_ms (headers wait) plus one between_bytes_timeout (worst-case first-body-chunk read), one-shot. Footnote 1 single-send section + section 5.4 test row updated. - OutboundResponse::into_body() added as the app-facing consuming accessor for streamed-response orchestration. The send_all rustdoc recommends single send + futures::join_all + into_body() on Axum/CF/Spin as the canonical path; into_parts(..) stays adapter-facing. Appendix AY records the five resolutions. Status header bumped to "rounds 1-50, Date: 2026-06-08"; superseded-AR pointer extended to include AY.
Contributor
Author
Round 50 — Fastly SDK correctness pass (commit
|
Five round-50 carry-over findings: - Early section 4.3 dynamic-backend prose still preserved the stale identical-properties-re-register carve-out, contradicting the corrected step-5 algorithm. Rewritten in place to match the unconditional session-uniqueness contract. Two historical appendix entries (round-37 in Appendix AK) marked superseded by Appendix AY. - Fastly buffered-fallback for lazy passthrough named max_response_bytes as the cap, but that per-request cap is unavailable at response-converter time. Added FASTLY_RESPONSE_STREAM_BUFFER_BYTES adapter-level constant (mirrors AXUM_RESPONSE_STREAM_BUFFER_BYTES). Three section 5.4 rows rebucketed so Fastly is no longer in the CF/Spin lazy group; new Axum-and-Fastly buffered-fallback row carries both adapter constants. - Residual between_bytes_timeout write-side claim removed from the remaining section 5.4 stalled-upload mechanics row and from section 8 risk 7. Fastly write phase is BestEffort uniformly now; the public Backend API docs are cited as the source. - Spin host-write race rewritten against actual WASI output-stream semantics. Old wording said each write() is raced against a timer; WASI write() is nonblocking and readiness-polled, so the implementable pattern is subscribe-pollable + futures::select! vs timer + nonblocking check_write() + write() within the permitted byte count. - Typo: "docsare migrated" -> "docs are migrated" in section 1.3 non-goals. Status header bumped to rounds 1-51; AR-superseded pointer extended to include AZ.
Contributor
Author
Round 51 — round-50 carry-overs + Spin WASI write mechanics (commit
|
aram356
force-pushed
the
feature/extensible-cli
branch
2 times, most recently
from
June 22, 2026 19:24
161a244 to
93f72fe
Compare
# Conflicts: # .github/workflows/test.yml # .tool-versions # Cargo.lock # Cargo.toml # crates/edgezero-adapter-axum/src/cli.rs # crates/edgezero-adapter-axum/src/config_store.rs # crates/edgezero-adapter-axum/src/dev_server.rs # crates/edgezero-adapter-axum/src/service.rs # crates/edgezero-adapter-cloudflare/src/cli.rs # crates/edgezero-adapter-cloudflare/src/lib.rs # crates/edgezero-adapter-cloudflare/src/request.rs # crates/edgezero-adapter-cloudflare/src/templates/src/lib.rs.hbs # crates/edgezero-adapter-fastly/Cargo.toml # crates/edgezero-adapter-fastly/src/cli.rs # crates/edgezero-adapter-fastly/src/config_store.rs # crates/edgezero-adapter-fastly/src/lib.rs # crates/edgezero-adapter-fastly/src/request.rs # crates/edgezero-adapter-spin/src/cli.rs # crates/edgezero-adapter-spin/src/cli/push_cloud.rs # crates/edgezero-adapter-spin/src/request.rs # crates/edgezero-adapter-spin/src/templates/runtime-config.toml.hbs # crates/edgezero-adapter-spin/src/templates/spin.toml.hbs # crates/edgezero-adapter-spin/tests/contract.rs # crates/edgezero-adapter/src/registry.rs # crates/edgezero-cli/Cargo.toml # crates/edgezero-cli/src/adapter.rs # crates/edgezero-cli/src/args.rs # crates/edgezero-cli/src/config.rs # crates/edgezero-cli/src/generator.rs # crates/edgezero-cli/src/lib.rs # crates/edgezero-cli/src/main.rs # crates/edgezero-cli/src/provision.rs # crates/edgezero-cli/src/scaffold.rs # crates/edgezero-cli/src/templates/app/name.toml.hbs # crates/edgezero-cli/src/templates/cli/src/main.rs.hbs # crates/edgezero-cli/src/templates/core/Cargo.toml.hbs # crates/edgezero-cli/src/templates/core/src/config.rs.hbs # crates/edgezero-cli/src/templates/core/src/handlers.rs.hbs # crates/edgezero-cli/src/templates/root/Cargo.toml.hbs # crates/edgezero-cli/src/templates/root/README.md.hbs # crates/edgezero-cli/src/templates/root/edgezero.toml.hbs # crates/edgezero-cli/src/templates/root/gitignore.hbs # crates/edgezero-core/src/app.rs # crates/edgezero-core/src/app_config.rs # crates/edgezero-core/src/context.rs # crates/edgezero-core/src/env_config.rs # crates/edgezero-core/src/error.rs # crates/edgezero-core/src/extractor.rs # crates/edgezero-core/src/key_value_store.rs # crates/edgezero-core/src/lib.rs # crates/edgezero-core/src/manifest.rs # crates/edgezero-core/src/store_registry.rs # crates/edgezero-macros/src/app_config.rs # crates/edgezero-macros/tests/app_config_derive.rs # crates/edgezero-macros/tests/ui/secret_bogus_kind.stderr # crates/edgezero-macros/tests/ui/secret_with_serde_flatten.stderr # crates/edgezero-macros/tests/ui/secret_with_serde_skip_serializing_if.stderr # docs/.vitepress/config.mts # docs/guide/kv.md # docs/superpowers/plans/2026-05-20-cli-extensions.md # docs/superpowers/plans/2026-06-01-spin-kv-backed-config.md # docs/superpowers/plans/2026-06-04-spin-per-backend-push.md # docs/superpowers/specs/2026-06-01-spin-kv-config.md # examples/app-demo/Cargo.lock # examples/app-demo/app-demo.toml # examples/app-demo/crates/app-demo-adapter-fastly/fastly.toml # examples/app-demo/crates/app-demo-adapter-spin/spin.toml # examples/app-demo/crates/app-demo-cli/Cargo.toml # examples/app-demo/crates/app-demo-cli/src/main.rs # examples/app-demo/crates/app-demo-cli/tests/config_flow.rs # examples/app-demo/crates/app-demo-core/src/config.rs # examples/app-demo/crates/app-demo-core/src/handlers.rs # examples/app-demo/edgezero.toml # scripts/smoke_test_config.sh
- Define config-store / kv-store / secret-store capability semantics (bare enum variants had no doc comment, unlike the six outbound caps). - Add explicit transport-error mapping bullets to the Cloudflare (§4.2) and Spin (§4.4) adapters, matching Axum/Fastly (DNS/TLS/connect -> bad_gateway). - Collapse the now-obsolete dual-baseline enforcement-gate hedging: PR #269 has merged to main, so the #269 sibling-gate topology is the active shape and the pre-#269 wording is marked historical.
- §4.3: document Fastly send_all cancellation/drop semantics (no async cancellation; every PendingRequest is harvested; sibling deadline never aborts other slots). - §3.3.1 / §3.1.3: spell out Deadline::after overflow fallback and into_response()'s only Err (adapter-invariant internal) condition. - §5.4: add test rows for Json/ValidatedJson cap migration, the *Within explicit-cap extractors, the per-adapter capability() matrix, and back-compat manifest parse. - §5.5: state where Tier 2 runs per adapter (host gate for Axum; per-adapter wasm-target matrix for the three WASM adapters). - §7: add dispositions for adapter request.rs, the Tier 3 MockServer, the run_* gate-site files + ensure_capabilities home, and CLAUDE.md/workflow wasip2 refresh; note sha2 is already a workspace dep. - Collapse the now-obsolete dual-baseline (#269 pre/post) framing in §3.5.2, §5.5, and §6 to the post-#269 shape, keeping the pre-#269 note as history.
- into_response(): drop the unreachable 'body handle surrendered by a prior terminal call' Err example — all terminal methods take self by move, so double-consumption is a compile error; reword as infallible-in-safe-use with a reserved internal path. - §5.4 capability() matrix row: include the omitted BoundedCooperative variant (CapabilitySupport has four variants: Native / BoundedCooperative / BestEffort / Unsupported).
…er (WIP) Amends the spec from three source-verified investigations (spin-sdk 6/wasip3, worker 0.8.3/workerd, Spin host-sync lifecycle) plus review findings: - Spin: replace the unimplementable WASI-0.2 upload loop (wasi:io is deleted in WASI 0.3) with a hand-built wasi:http request; the SDK's high-level send spawns an uncancellable body pump. Lazy response passthrough = BestEffort (EdgeZero's own FullBody alias, not a platform limit). - Cloudflare: set-cookie IS preservable (compat_date already enables it); the collapse is HeaderMap::insert. Second collapse bug on the client-facing path (Headers::set) and a to_str().unwrap() panic hazard. Comma-joining of repeated non-set-cookie headers is the real, narrow loss. - Spin host sync: provision writes, build/serve/deploy validate at adapter.rs::execute (a hook in SpinCliAdapter::execute would be dead code). - Fastly SendErrorCause -> 504/502; RequestContext::new preserved; test seams behind test-utils (not cfg(test)); tiers redefined by owning crate. Known-open: a follow-up review found compile-level errors in the normative pseudocode (pub(crate) validator across crates, builder/getter name collisions, Spin pump result handling, StoredError reconstruction, Hooks defaults vs denied missing_trait_methods). Tracked in the decisions register; NOT ready for task-level planning.
…cs/outbound-http-spec
…ted)
Built a 2-crate compile-check skeleton (core + separate adapter crate) and let
the compiler/clippy adjudicate the review's compile-class findings:
- validate_for_dispatch: pub (pub(crate) is unreachable from adapter crates) [E0603]
- deadline accessors: single budget_inputs() struct accessor (getter names
collided with the public builder setters -> E0592 duplicate definitions)
- Spin pump: unwrap Result<Bytes,EdgeError>, keep+prioritize pump result
(join!(...).1 silently discarded source errors and cap overflow)
- StoredError: variant-specific snapshot enum ({kind,message} doubled Internal's
prefix and couldn't hold ConfigOutOfDate/MethodNotAllowed/NotFound payloads)
- Capability schema: inline in manifest.rs + derive Serialize (reconciled active
section with §7; separate capability.rs won't compile in the macro include)
- Baked manifest: app! must emit BOTH Hooks methods (clippy::missing_trait_methods
is denied -> proven error); manifest() uses a per-impl function-local OnceLock
calling a PUBLIC Manifest::from_baked_json (reparse must call pub(crate) finalize,
unreachable from the downstream generated impl)
- Spin host default: Option<Vec<String>> (absent=None https-only vs explicit
["*"]=http+https; bare Vec can't carry the distinction -- serde test passes)
- Fastly cache: MUST be session-scoped thread_local!, not a per-request client
field (client is constructed per inbound request; names are session-global ->
request #2 fails closed on NameInUse)
- provision: corrected -- receives NO host data; needs manifest re-read or a
signature change (my earlier 'no trait change' was wrong)
- config validate: run_config_validate_typed is a separate public entry generated
CLIs call directly; gate must be a shared inner op behind both entries
- Fastly test seam: recording BackendBuilder, not a cache inspector (Backend is
opaque -- getters don't round-trip SSL/SNI/cert/override_host)
Skeleton at scratchpad/skel compiles green in corrected form. Still open: policy
calls (loopback-vs-no-network, command-class gating, compression, migration
completeness incl PROXY_HEADER + error-JSON status field).
Skeleton (scratchpad/skel, compiles green) confirmed the CLI-gate wiring shape:
a gate on only one of run_config_validate / run_config_validate_typed leaves the
typed path silently ungated -> must be a shared inner gated op both entries call.
Policy decisions (all user-locked):
- Command-class gating: gate runtime-producing/mutating (build/serve/deploy/
provision/config push) + validation (config validate); EXEMPT read-only
diagnostic (config diff, auth status) and credential (auth login/logout). This
REVERSES the earlier 'gate config diff' lock. Gate count 6 -> 5; execute(..)
branches to skip auth. Swept §3.5.3 table, §5.4 enumerations (+ exemption
regression-guard row), §6/§7 counts, §1 header.
- Loopback tests: amend CLAUDE.md so 'network' = public internet; a loopback
mock origin is permitted (in-process fake rejected -- can't prove real socket
concurrency).
- Compression: decode single gzip/br; identity/unknown/stacked/repeated ->
passthrough untouched (bytes + content-encoding intact), never a hard fail.
Case-insensitive token match. Normative table added to §3.4.1.
- PROXY_HEADER (x-edgezero-proxy): PRESERVED through the rename.
Migration honesty (finding 13): replaced the false 'no public capability lost'
claim with an enumerated removal table -- ProxyHandle::client(), body_mut(),
extensions()/extensions_mut(), request_mut() are DROPPED (breaking); PROXY_HEADER
preserved. EdgeError JSON shape corrected to the REAL converter's
{status, kind, message, field_path?}, not {kind, message}.
…e appendices) Independent source-grounded verification review: compile-class CLEAN (no blockers), current-tree claims ALL accurate. Two active defects were my own incomplete command-class-gating sweep from the prior round, plus three stale appendix entries: - §7 CLI gate enumerations (both) still gated the EXEMPT config diff and said execute(..) covers auth. Corrected: execute gates build/serve/deploy only (branches to skip Auth*); four siblings run_provision/run_config_push_typed/ run_config_validate/run_demo; run_config_diff_typed is NOT a gate site. - §5.4 row asserted the execute gate fires on auth -> corrected to build/serve/ deploy only, + asserts config diff / auth do NOT hard-fail. - Appendix entries: added [SUPERSEDED] markers (not rewriting frozen history, matching the spec's existing 'superseded by AY' convention) to: lazy=Native- on-three-WASM-adapters (only CF Native now), the WASI-0.2 subscribe/check_write protocol (doesn't exist in SDK 6), and the #[cfg(test)] seam (must be test-utils). Also compile-verified the Fastly session-cache thread_local! pattern in the skeleton (request #2 reuses across per-request clients; mismatch fails closed). Gate count now consistently FIVE across §3.5.3, §5.4, §6, §7; auth + config diff exempt everywhere in the authoritative body.
…tives)
First executable TDD slice of the outbound-http implementation. Scoped to the two
purely-additive, skeleton-verified core primitives that keep cargo test --workspace
green at every step:
- EdgeError::BadGateway (502) / GatewayTimeout (504): variants, constructors, all
five exhaustive-match updates (kind_str/message/status/inner/into_response
field_path), + tests for status/kind/message and the {error:{status,kind,message}}
JSON shape (no field_path). Test code verified against the real error.rs APIs
(sync into_bytes(), nested error-wrapping).
- time.rs: Deadline (after with DEADLINE_FAR_FUTURE clamp + saturating checked_add,
at_instant/instant/remaining/is_expired), DispatchBudget struct, and the three
budget constants with exact verbatim values.
dispatch_budget() itself is explicitly deferred to Phase 1b (needs OutboundRequest +
the budget_inputs() accessor), which is the breaking proxy->outbound rename slice.
PLAN (all findings verified against the real tree before fixing): - Task 1 would NOT have compiled: enumerated only 5 exhaustive EdgeError matches, but there are EIGHT -- the 3 test-module matches (error.rs 281/335/369) have explicit panic-arms listing every other variant with NO '_' wildcard, so adding two variants breaks them. #[non_exhaustive] does not relax exhaustiveness inside the defining crate. Listed all 8 + added a compiler-driven catch step (E0004). - is_expired() was WRONG at exact equality: checked_duration_since returns Some(ZERO) at t==deadline, so it reported not-expired, contradicting the spec's 'deadline <= now => expired'. Now compares instants directly; remaining() returns None at equality. Added an equality test. - 'cargo test -p X a b' is invalid (verified: unexpected argument 'b'). Single filter. - GatewayTimeout's JSON contract was untested -> both variants table-driven, and the existing kind_strings_per_variant matrix (error.rs:502) gains rows (with the suffixed 502_u16/504_u16 literals the macro actually uses). - Flaky timing tests replaced with instant-bounded ones (no now()-1s underflow, no 50-60s tolerance); the Duration::MAX test now actually proves the 7-day clamp. - DispatchBudget moved to Phase 1b to ship with its producer (spec treats §3.3.2 carrier+producer as one contract). - fmt added to Task 1; Task 3 now runs all five CI gates incl. fmt --check and the wasm32-wasip2 target. - Phase 1b noted as multiple slices, not one atomic step. SPEC (stale pseudocode/footnotes the prose sweeps missed): - Fastly cache ownership normalized to ONE owner: session-scoped thread_local! + RefCell (single-threaded guest, no Mutex). Removed the contradictory per-client 'Mutex<HashMap> on FastlyOutboundClient, one per request context' claims, which fail on the 2nd inbound request (names are session-global, client is per-request). Test seam targets the thread-local, not a client field. - Footnote 2 claimed between_bytes_timeout bounds streamed-upload inter-chunk gaps; it is receive-side only and bounds nothing on the guest->origin write path. Now agrees with §4.3 and §8 risk 7: BOTH source-pull and host-write are BestEffort. - CLI gate pseudocode rewritten: execute(..) branches to skip Auth*, gates the TYPED push path (bundled run_config_push is an erroring stub), validate is adapter-less and loops configured adapters behind ONE shared gated_validate that both the bundled and typed entries call, demo reads the baked Hooks::manifest(), and run_config_diff_typed is explicitly NOT a gate site. - Spin sample applied '?' to Option<Duration> in a Result fn -> explicit let-else to gateway_timeout.
…r stages
Verified every finding against the tree/compiler before fixing.
PROVEN BUGS (skeleton-adjudicated):
- Trait-default static is NOT per-impl. Proof test: two Hooks impls relying on a
caching default BOTH returned the first impl's value (AppA=1, AppB=1) -- items
in generic fns are not monomorphized, so ONE static is shared by all
implementors. As specced, app A's baked manifest would serve app B's capability
checks. Fix: default manifest() returns None; the OnceLock lives in each
macro-generated impl (distinct static per impl). Test kept as a regression guard.
- The repo DENIES clippy::restriction wholesale (root Cargo.toml) and does NOT
allow-list missing_inline_in_public_items / min_ident_chars /
arithmetic_side_effects / expect_used / as_conversions. My Phase 1a Deadline
code and the spec's pseudocode both violated it:
* plan: added #[inline] to every public fn, d -> duration, replaced
with checked_duration_since + explicit zero filter via pure remaining_at(now)/
is_expired_at(now) helpers (which also make the tests exact/deterministic --
no tolerance windows, no now()-1s underflow; the clamp test now bounds
d.instant() to prove the 7-day clamp).
* clippy.toml allows expect/unwrap/panic IN TESTS but NOT arithmetic_side_effects
-- documented, hence checked_add(..).expect(..) in test code.
* spec: dispatch_budget .expect(..) -> explicit invariant EdgeError::internal;
fastly_timeout_ms casts + unchecked arith -> as_millis + saturating +
u64::try_from (compile-verified AND behaviour-tested: sub-ms->1, 1.5ms->2,
Duration::MAX clamps without panic).
FASTLY CONTRADICTIONS:
- Step 6 listed DNS/TLS as Backend::builder().finish() errors; they only arrive from
send. Split the two stages (BackendCreationError vs SendErrorCause), which also
makes the §5.4 rows writable (a fake builder cannot produce a DNS branch).
- Decided the ambiguous cause: DnsTimeout -> 504 (classify by 'did a timer fire?',
not by subsystem); DNS resolution failure (NXDOMAIN) -> 502.
- Canonical-accessor test row pointed at map inspection; Backend is opaque -> now
targets the recording BackendBuilder (map inspection reserved for identity/reuse).
- Purged mutex-era reasoning from the thread_local algorithm ('drop the lock' ->
'release the borrow'); removed the impossible 'prior session / another deployment'
NameInUse cause (names are session-scoped and may overlap across sessions) and
replaced it with the three real causes.
- Upload summary still claimed a BoundedCooperative write-side bound and that only
source-yield was the 'worst phase' -- both phases are unbounded BestEffort.
- [SUPERSEDED] markers on the two appendix Mutex-era entries.
STALE FOLLOW-UPS (verified already done -- would have scheduled redundant work):
- Risk 10 (Spin wasip2 refresh) CLOSED: CLAUDE.md already quotes wasip2 for gate 5
and lists Spin as wasm32-wasip2; surviving wasip1 refs are Fastly's (correct).
- §1 header no longer claims the CLI gates are unreconciled -- §3.5.3 reconciles them.
- §7 CLAUDE.md entry now carries the real deliverable: amend the unqualified
no-network rule to permit a loopback mock origin.
…rror policy Reviewer compiled the Phase 1a code under the repo's lint policy and found duration_suboptimal_units. I reproduced the repo's EXACT lint table in an isolated crate and found MORE than reported -- then iterated to green. PHASE 1a PLAN (now verified GREEN under the repo's real gate): - duration_suboptimal_units (pedantic; CI runs -D warnings so it FAILS the build): from_secs(7*24*60*60) -> from_hours(168); from_secs(60) -> from_mins(1). - arbitrary_source_item_ordering (denied restriction) -- NOT in the review, found by compiling: items must be ALPHABETICAL, and it policies ENUM VARIANTS too (proven: appending BadGateway after Validation errors). The plan said "add after Validation", which would have failed on first build. Corrected: BadGateway before BadRequest, GatewayTimeout between ConfigOutOfDate and Internal; constructors likewise. Deadline's methods reordered alphabetically. - Confirmed std_instead_of_core/std_instead_of_alloc ARE allow-listed, so `use std::time::Duration` is fine (checked the full allow-list rather than guessing). - Fixed the two "compares instants directly" descriptions -- impl uses checked_duration_since(..).filter(..). SPEC (High): - ManifestCapabilities/ManifestOutboundCapability derived only Deserialize while Manifest derives Serialize -> would break Manifest's derive and the macro crate. Both now derive Serialize. - SECURITY: hosts was Vec<String> defaulting to ["*"], but the renderer defines "*" as http+https while an ABSENT field must stay https-only. Every existing manifest that never declared hosts would silently gain cleartext outbound on next build. Now Option<Vec<String>>: None -> ["https://*:*"]; Some(["*"]) -> http+https (opt-in); Some([]) -> rejected. Validator signature updated to the inner slice. - Fastly error policy: replaced the two catch-alls with an EXHAUSTIVE per-variant table built from the real fastly-0.12.1 enums. Key correction: ConnectTimeoutTooLarge /FirstByteTimeoutTooLarge/BetweenBytesTimeoutTooLarge/NameTooLong/EncodingError and HttpRequestUriInvalid/InternalError mean EDGEZERO violated its own clamp/name/URI invariants -> `internal` (500), NOT bad_gateway. Mapping them to 502 disguises an adapter bug as an upstream failure. Widened the §5.4 "internal only on three paths" assertion to five, with a regression guard against the 502 mapping returning.
…t sweep)
Addresses ALL remaining feedback. Verified each against the real SDK/tree.
SECURITY -- baked-manifest gate failed OPEN:
- parse-failure and "no manifest" both collapsed to None, and ensure_capabilities
treats None as permission to proceed => a MALFORMED baked contract silently
disabled required-capability enforcement (the gate reports success exactly when
it can no longer verify anything). Replaced Option with a three-state
BakedManifest { Absent, Present(&'static Manifest), Malformed(&'static str) }:
Absent proceeds (no contract), Malformed HARD-FAILS with an actionable message.
- Macro output now uses fully-qualified paths (::edgezero_core::manifest::Manifest,
::std::sync::OnceLock) -- it expands in a downstream crate that may not have them
in scope.
FASTLY -- test seam was impossible as specced (verified against fastly-0.12.1):
- SendError has private fields + no public ctor; SendErrorCause is #[non_exhaustive].
NEITHER is constructible in a test, so "one test per SendErrorCause" was unwritable.
Also retracted my own earlier instruction to "match SendErrorCause exhaustively so a
future variant is a compile error" -- #[non_exhaustive] makes that impossible; a `_`
arm is mandatory. BackendCreationError is the opposite (not non_exhaustive =>
constructible AND exhaustively matchable). Documented the asymmetry and split
classification: a pure classify(SendFailure) over a locally-defined constructible
enum (unit-testable), plus a thin cause_to_failure boundary map covered by Tier 3.
- Purged the last mutex-era reasoning: "prior session" NameInUse cause (names are
session-scoped; a fresh session starts clean), "outer lock held continuously",
"no other thread", finer-grained locking. Now stated in single-threaded
session-map-borrow terms. [SUPERSEDED] marker added to the last appendix entry.
CLI:
- Manifest::configured_adapter_names() does not exist -> iterate
manifest.adapters.keys() (BTreeMap<String, ManifestAdapter>, manifest.rs:95;
ordered => deterministic failure reporting).
- The three config commands are NOT symmetric; replaced the over-general "same
applies to push/diff" with a per-command table: push = gate the TYPED path only
(bundled is an erroring stub, nothing to gate); validate = shared inner op both
entries call (the only one needing it); diff = never gated.
LINT SWEEP (spec pseudocode, same gate as Phase 1a):
- sent += chunk.len() -> saturating_add (arithmetic_side_effects).
- production cert_host().unwrap() -> explicit if-let (unwrap_used).
- dispatch_budget: added #[inline] + the lint contract; |dur| -> |duration|
(min_ident_chars), including the closure body.
PHASE 1a PLAN:
- The new variants must also join the retry_after_* and field_path_* matrices
(502/504 carry neither), not just kind_strings_per_variant -- three matrices.
…+ strip meta-refs from code comments
Verified each finding against the tree/compiler before fixing.
HIGH (self-inflicted lifetime bug): ensure_capabilities took BakedManifest, whose
Present holds &'static Manifest, but the four file-backed gate sites pass LOCAL
&Manifest borrows (not 'static) -> cannot compile. Introduced a lifetime-bearing
ManifestContract<'a> { None, Malformed(&'static str), Present(&'a Manifest) };
BakedManifest converts in via as_contract(); file-backed callers build it from
Option via from_opt. Fail-closed on Malformed preserved.
HIGH (StoredError): the "total" capture omitted NotImplemented + ServiceUnavailable
(EdgeError has 10 variants); MethodNotAllowed stored String not Method (forcing a
fallible reconstruction); the cancellation example used the REJECTED flat
{kind,message} shape; and three call sites used clone_as_edge_error() while the
method is to_edge_error(). All fixed.
HIGH (Fastly error classification, mutually-exclusive rules): creation step-6 prose
+ the §5.4 stage-1 row said "every non-NameInUse -> 502", contradicting the
per-variant table (clamp/name/encoding -> internal 500). And a "internal is never
correct during send" sentence contradicted SendFailure::LocalInvariant. The
per-variant tables are now authoritative everywhere; the blanket-502 prose is gone.
HIGH (Fastly cache unbounded): backend identity used EXACT ceiled budget_ms, which
drifts by the ms across requests (97/98/99 for a nominal 100ms deadline) while the
thread_local map is session-wide -> one leaked backend per distinct ms, unbounded.
Now identity uses budget_bucket_ms (ceil to a 1-2-5 grid); host timers armed with
the bucket (>= real budget, a looser backstop) while the exact deadline stays
enforced by the cooperative is_expired() check. Bounds distinct backends per host to
~a few dozen, independent of request count. Added a bound-assertion §5.4 row.
HIGH (Spin provisioning wrong-file): recommended re-reading from manifest_root, but
manifest_root = args.manifest.parent() DROPS the filename while --manifest accepts an
arbitrary name -> would read the wrong file for --manifest custom.toml. Now: thread
the hosts (or full path) into provision; a bare re-read does not work.
HIGH (outbound-host default): the initial manifest EXAMPLE still advertised the
cleartext-enabling ["*"] default the corrected schema rejects. Example + appendix now
state absent -> https-only ["https://*:*"], explicit ["*"] -> http+https opt-in.
MEDIUM: Manifest::configured_adapter_names() doesn't exist -> iterate
manifest.adapters.keys() (real BTreeMap field). The three config commands aren't
symmetric (per-command table). Response migration summary said buffering is reserved
for Body::Once on all three WASM adapters -- only Cloudflare streams lazily; Axum,
Fastly, Spin all buffer Body::Stream (fixed + appendix superseded marker).
MASTER-SPEC LINT: DEADLINE_FAR_FUTURE from_secs(7*24*60*60) -> from_hours(168) at both
live sites; Capability enum carries a documented #[expect(arbitrary_source_item_ordering)]
(semantic grouping tied to the matrix); added a prominent §3 directive that all code
blocks are illustrative and must be brought to the strict lint gate before use.
PHASE 1a PLAN: `let v` -> `body_json` (min_ident_chars, reviewer-verified); the three
error.rs tests are not all "exhaustive matrices" (only kind_strings_per_variant is;
the other two are subset checks) -- wording corrected. Deadline block replaced with
the version compile-verified GREEN under the repo's exact lint table.
COMMENTS: stripped section-number / round-N / phase meta-references from all code-block
comments (plan + 894 spec lines) -- comments describe the code, not the planning process.
…consistency Independent review verified the Phase 1a plan end-to-end against the tree and found one real defect, now confirmed here: - The Task 1 / Task 2 test snippets did NOT pass cargo fmt --check as written. Verified with rustfmt at the REAL test-module nesting (inside ): the one-line array-of-tuples rows exceed max_width=100 (~106 cols) and rustfmt rewraps every tuple element onto its own line, plus the message-bearing assert!/assert_eq! and checked_add(..).expect(..) chains. The plan presented these as exact landed code and its Self-Review claimed 'no diff' -- wrong. Replaced both snippets with their rustfmt-canonical multi-line form (re-derived from rustfmt, not guessed); rustfmt --check now exits 0 at real nesting for both blocks. - Spec appendix nit: the resolved-findings table still wrote DEADLINE_FAR_FUTURE = Duration::from_secs(7 * 24 * 60 * 60), contradicting the canonical §3.3.1 def (from_hours(168), which the live sites already use). Made consistent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the EdgeZero outbound HTTP design spec at
docs/superpowers/specs/2026-05-21-outbound-http-design.md. Targets the PR #269 (feature/extensible-cli) baseline — the spec assumes the multi-store manifest, theedgezero_cli::adapter::execute(..)dispatcher, the expandedAdapterActionset,Adapter::provision/ config-validation hooks, Spin SDK 6 / wasip2, and thedemocommand. Earlier appendices that quote the pre-#269 surface are explicitly flagged as historical.Driving pattern
The spec is written against fan-out HTTP workloads — N concurrent outbound requests under a shared wall-clock deadline, results harvested in input order. The driving pattern is treated as a portable substrate; the spec deliberately does not name a single consumer.
Scope
Six driver requirements:
OutboundHttpClienttrait with singlesendand concurrentsend_allon every adapter (Axum, Cloudflare, Fastly, Spin). One handler source compiles unchanged across all four.dispatch_budget(req, now)with explicitnowsnapshot,DEFAULT_NO_DEADLINE_BUDGET = 30s,DEADLINE_FAR_FUTURE = 7 days,BATCH_DISPATCH_SLACK_MAX = 25ms. Fastly's bounded-cooperative semantics are documented with precise overshoot bounds.max + sizeof(current_chunk)worst case, in-flight chunk size source-controlled), pre-append cap checks across inbound + outbound bounded drains. Batch model isΣᵢ request_bodyᵢ.len() + Σᵢ max_response_bytesᵢ.outbound-http,outbound-deadlines,outbound-flexible-phase-budget,send-all-slot-isolation,streamed-upload-deadlines,lazy-streamed-response-passthrough,config-store,kv-store,secret-store). Enforcement runs as five pre-dispatch gates (one insideexecute(..), siblings onrun_provision/run_config_push/run_config_validate/run_demo).MockOutboundClient), Tier 2 (per-adapter translation), Tier 3 (runtime against a local mock origin). Adapter-specific mechanics (Fastly host timers, harvest behaviour, dynamic backend identity) are restricted to Tier 2/3 since Tier 1's mock has no analogue.backend_target() / host_authority() / sni_hostname() / cert_host()are the single source of truth for the host/port/SNI/cert split. Adapters MUST consume these, not re-derive fromreq.uri(). IP-literal HTTPS (RFC 6066 §3) is handled bysni_hostname() == None && cert_host() == Some(ip).Out of scope (explicit non-goals)
tokio,reqwest,fastly,worker, orspin-sdkin core or app/library crates.Process
The spec was revised through 49 review rounds. The non-normative resolution journal lives in Appendices A through AX. Appendix AR is the round-44 rebase snapshot, superseded by Appendices AS / AT / AU / AV / AW / AX (rounds 44–49).
Test plan
This is a docs-only PR.
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-features -- -D warningscargo test --workspace --all-targetsdocs/)