EdgeZero CLI Extensions: extensible CLI, multi-store manifest, auth/provision/config#269
Open
aram356 wants to merge 268 commits into
Open
EdgeZero CLI Extensions: extensible CLI, multi-store manifest, auth/provision/config#269aram356 wants to merge 268 commits into
aram356 wants to merge 268 commits into
Conversation
… 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.
Convert the http builder re-exports to `pub type` aliases (real fix — no `pub use` required) and wrap the `header` re-export in a child module with a scoped `#![expect]`. Add file-level `#![expect(clippy::pub_use)]` to each adapter lib.rs (axum, fastly, spin, cloudflare) and to edgezero-core/lib.rs for the proc-macro re-export. Cloudflare uses `cfg_attr(target_arch = "wasm32", expect)` because its re-exports are wasm-gated and would leave the expect unfulfilled on the host build.
For each adapter (axum/fastly/spin/cloudflare): make the previously
private internal modules `pub mod` and drop every `pub use` re-export.
Callers now reach types via the full path, which is what the lint
suggests as the proper fix. Update internal cross-module refs and
external callers (edgezero-cli, demo crates, axum/spin scaffold
templates, fastly/spin/cloudflare contract tests).
Remaining `pub_use` expects:
- `edgezero-core/src/lib.rs` — single-line proc-macro re-export
(`pub use edgezero_macros::{action, app}`); the canonical proc-macro
distribution pattern requires this and the lint is module-scoped, so
a tightly-scoped file-level expect is the only available form
- `edgezero-core/src/http.rs::header` — wrapped in a child module with
the expect scoped to that one line; required by the CLAUDE.md HTTP
facade rule
The two `#[allow(deprecated)]` annotations on `AppExt::dispatch` implementations (cloudflare/fastly) were unnecessary — implementing a deprecated trait method does not trigger the `deprecated` lint, only calling the deprecated declaration does. Drop them. Also fix the fastly contract integration test (wasm32-only) which was still importing names from the previous crate-root re-exports — switch to the new `request::`/`response::`/`context::` module paths.
Per-package `.cargo/config.toml` is only honored when cwd is inside the package directory, so `cargo test -p edgezero-adapter-fastly --target wasm32-wasip1 --test contract` from the workspace root fails to resolve the Viceroy runner. Mirror the runner at the workspace level. Cargo invokes test runners with cwd set to the package manifest directory, so `../../examples/...` resolves correctly for any adapter package targeting wasm32-wasip1.
Three previously-duplicated wasm test jobs (cloudflare, fastly, and a new spin entry) collapse into one `adapter-wasm-tests` matrix that varies on adapter, target, and runner. Spin uses Wasmtime; fastly keeps Viceroy; cloudflare keeps wasm-bindgen-test-runner. Per-adapter toolchain installs are gated with `if: matrix.adapter == ...` so each job only pulls what it needs. Also fix a pre-existing compile error in `crates/edgezero-adapter-spin/ tests/contract.rs:171` (`name == "x-edgezero-res"` needed a deref) — silently broken because there was no CI job exercising it.
Remove the `extra_check` matrix flag and gating `if:` — every adapter in the wasm matrix now runs the same test+check pair, and the duplicate "Check Spin wasm32 compilation" step in the top-level `test` job (now redundant with the matrix's spin cell) goes away. axum is the host-target adapter — its 102 tests already run as part of `cargo test --workspace --all-targets` in the `test` job. It has no `--test contract` integration target, so adding it to the wasm matrix would either need a special-case command or duplicate the workspace-test work. Keeping it in the `test` job is the simpler call.
The cargo cache restores `~/.cargo/bin/{viceroy,wasm-bindgen-test-runner}`
from prior runs; a bare `cargo install` then fails with `binary already
exists in destination`. Match the same `command -v` guard the spin step
already uses, and for wasm-bindgen also re-check the version (the cache
key is per-Cargo.lock so a wasm-bindgen bump in lockfile needs a refresh).
Replace the conditional `command -v` guards with unconditional `cargo install --force` for both viceroy and wasm-bindgen-cli. The cargo cache restores prior binaries into `~/.cargo/bin/` and `cargo install` rejects by default; the previous version-grep guard was fragile and the simpler `--force` is always safe with `--locked`.
Five `pub(crate)` items in fastly/spin are file-local, not actually cross-module: drop them to private (Stores, dispatch_with_handles, resolve_kv_handle, resolve_secret_handle, MAX_DECOMPRESSED_SIZE). Also drop `validate_name` to private in edgezero-core/secret_store (only used inside the same file). The remaining five `pub(crate)` items (dispatch_raw, dispatch_with_ store_names, parse_uri, parse_client_addr, decompress_body) are genuine cross-file crate-internal API and must stay at crate visibility. `pub_with_shorthand` wants `pub(in crate)` but rustfmt unconditionally rewrites that back to `pub(crate)` — there is no spelling that satisfies both the lint and rustfmt, so the workspace allow stays with a tighter rationale.
For every previously inline `std::*`, `fastly::*`, `crate::*` etc.
absolute path, add a `use` import at the appropriate scope (file top
or `mod tests {}`) and replace the inline path with the short name.
Affects ~30 files across edgezero-core, all four adapters, and the
CLI. No behaviour change; lint count down by one workspace allow.
Reorder source items across edgezero-core and the adapter/cli crates
to satisfy the canonical clippy item ordering (ExternCrate → Use → Mod
→ Static → Const → TyAlias → Enum → Struct → Trait → Impl → Fn) with
alphabetical ordering inside each kind. Applies recursively to:
- top-level items in 12 core files (app, body, config_store, context,
error, extractor, http, key_value_store, middleware, params, proxy,
router, secret_store) and the adapter/cli files that needed it
- struct fields and constructor argument order
- enum variants
- methods inside `impl` blocks
- items inside `mod tests {}` blocks (including macro_rules! placement
before `use super::*` where required)
Pure reordering — no behavioural changes, no `#[expect]` annotations.
All clippy lints pass, 557+ tests green, all three wasm targets compile.
All cast sites turned out to be either redundant trait-object coercions that Rust performs automatically, or numeric conversions that can use a sibling const at the right type: - spin/decompress.rs (2 sites): added MAX_DECOMPRESSED_SIZE_U64 sibling const so the `Read::take` callsites do not need a usize→u64 cast - fastly/logger.rs: replaced `Box::new(logger) as Box<dyn log::Log>` with an inline `let boxed: Box<dyn log::Log> = Box::new(logger);` pattern (Box<T>→Box<dyn Trait> coerces automatically through a typed binding) - core/middleware.rs (4 sites in tests) and core/router.rs (1 site): same pattern — drop redundant `as BoxMiddleware` casts where the surrounding `Vec<BoxMiddleware>` annotation already drives coercion - cli/main.rs: drop `&[] as &[String]` — the function signature drives inference Workspace allow is gone; clippy + 557+ tests + all wasm targets pass.
Six arithmetic sites — all on usize/SystemTime where overflow is practically impossible but the lint cannot prove it. Real fix: use the explicit no-panic variant at each site. - axum/key_value_store.rs: `limit + 1` → `limit.saturating_add(1)`, `MAX_SCAN_BATCHES * LIST_SCAN_BATCH_SIZE` → `saturating_mul`, `batch_count += 1` → `saturating_add`, and `SystemTime::now() + ttl` → `SystemTime::now().checked_add(ttl).ok_or_else(KvError::Internal)?` so an absurd ttl propagates as an error rather than panicking - core/key_value_store.rs (test MockStore): same `checked_add(ttl)?` pattern so the test backend matches the production contract - cli/generator.rs: `count + 1` → `saturating_add(1)` Workspace allow gone; all clippy lints, tests, and wasm targets pass.
viceroy 0.17.0 raises its MSRV to rustc 1.95; the workspace ships rustc 1.91 (.tool-versions), so the unpinned `cargo install viceroy` started failing with "rustc 1.91.1 is not supported by viceroy-lib@0.17.0 requires rustc 1.95". 0.16.x is compatible and is what local dev uses.
Matches the CI pin (`^0.16`) so local dev resolves the same major.minor that CI installs. 0.17 raises MSRV to rustc 1.95 which is past the workspace's rust 1.91.1.
Single source of truth: replace the hardcoded `^0.16` in the workflow with a step that greps the version out of `.tool-versions`. Matches the existing pattern used for rust, and means a future viceroy bump is a single-line edit in `.tool-versions` rather than two places.
Single-character bindings, closure params, and helper variable names were renamed to descriptive equivalents across 31 files. Common patterns: - closure error params: `|e|` → `|err|` - closure key/value pairs: `|(k, v)|` → `|(key, value)|` - short locals in tests: `let s = ...` → `let store/service/cs = ...` - `Some(p)` for `&UserProfile` → `Some(found)` (avoids shadow with outer `profile` var, which would trip `shadow_reuse`) - `let h = handle.clone()` in concurrent tests → `let kv_handle = ...` to avoid shadowing the outer `handle` - `m` (manifest data) in dev_server.rs / main.rs → `manifest_data` - HTTP closure params `|c| c.get(...)` → `|http_client| http_client.get` No behaviour changes — pure renames. Workspace allow gone; clippy + 557+ tests + all wasm targets pass.
Investigated removing the allow: 40 sites in edgezero-core alone (every public error type and handle: EdgeError, KvError, SecretError, ConfigStoreError, ConfigStoreHandle, plus the entire Manifest* family). The renames would force consumers in 4 adapter crates + cli + demo to either write `kv::Error`/`secret::Error`/etc. at every callsite or set up `use ... as KvError` aliases — a net loss in readability for a deliberately-prefixed cross-crate API. Replaced the terse comment with a longer one documenting the audit and why the allow is load-bearing rather than a leftover.
… feature/extensible-cli PR #257 (chore/strict-clippy) was squash-merged into main as 7ec2ad1, so the same content this branch had been pulling in piecewise via repeated `origin/chore/strict-clippy` merges (c666132, 1b7342f, 3082391, a04814d, 9175cff, e526e6b) now arrives as a single tree-level diff against main. Tree-level reconciliation produced 54 conflicts because the squash commit ships the pre-rewrite/pre-PR-#269 shape of each affected file (old `Command::{ Build {…}, Deploy {…}, Serve {…}, Dev }` enum variants, `handle_build/deploy/serve` routing in `main.rs`, old `SimpleLogger` init, missing `Auth`/`Config`/`Provision` subcommands, old store/manifest types, etc.), while HEAD has the fully-rewritten state that PR #269 has been delivering and reviewing for the last several rounds. Resolution: take HEAD for all 54 conflicts. The squash commit adds nothing this branch doesn't already have via the prior piecewise merges — every conflict region's upstream side is content this branch explicitly rewrote. Confirmed by walking the conflict markers and matching each upstream hunk to a corresponding rewrite already on HEAD (case-insensitive adapter lookup, manual `Default` impls, `[stores]` `deny_unknown_fields`, `__` rejection in app-config keys, F1-F7 fastly/cloudflare/spin fixes, `.tool-versions` scaffold template, `CliLogger` in lib.rs, etc.). One additional new-file delete: `crates/edgezero-cli/src/dev_server.rs` (added by the squash) is the pre-rewrite location of the dev server; HEAD has moved it to `crates/edgezero-adapter-axum/src/dev_server.rs`. Removed the stray upstream-added file. Plus one real round-5 review-feedback fix carried over from upstream's `75a01ec strict-clippy: round-5 review feedback (cli args + logger)`. That commit had two fixes: 1. `args.rs`: drop the unused `--local-core` flag from `NewArgs`. This branch already doesn't have it (we dropped it as part of earlier rewrites), so it's already in the right state. 2. `lib.rs::CliLogger`: filter `debug`/`trace` explicitly rather than routing them to stdout alongside `info`. Pre-fix the match arm wrote `info|debug|trace => println!(...)` and the doc claimed all three went to stdout, but `enabled()` already capped at `<= Info` and `LevelFilter::Info` was set globally so debug/trace never reached `log()` — the comment promised behaviour the impl couldn't deliver. Split the match arm so `info => println!`, `debug | trace => {}`, and the doc now says "info to stdout, warn/error to stderr; debug/trace filtered out (no verbosity flag yet)". Applied here on lib.rs since this branch already moved `CliLogger` out of `main.rs` (where upstream put it) into the library so downstream CLIs built on `edgezero-cli` reuse the same prefix-less output. Verified on the merged tree (all gates green): - cargo fmt --all -- --check - cargo clippy --workspace --all-targets --all-features -- -D warnings - cargo test --workspace --all-targets - cargo clippy -p edgezero-adapter-spin --target wasm32-wasip2 --features spin --all-targets -- -D warnings - cargo clippy -p edgezero-adapter-fastly --target wasm32-wasip1 --features fastly --all-targets -- -D warnings - cargo clippy -p edgezero-adapter-cloudflare --target wasm32-unknown-unknown --features cloudflare --all-targets -- -D warnings - cd examples/app-demo && cargo test --workspace --all-targets
Hard-cutoff redesign of the typed app-config storage layer: single JSON blob per environment instead of per-leaf key=value writes, embedded SHA-256 for drift detection, per-environment KEY override, framework-resolved #[secret] fields (Model A), config diff command, syn-based nested-AppConfig CI gate, and an aligned-to-shipped Spin Cloud 96 KiB writer cap. Drafted across one self-review pass and sixteen reviewer passes; the spec's internal v1 changelog records the round-by-round evolution of every decision (storage shape, CLI args, stub-pointer UX, secret walk, canonicalisation, error variants, test plan).
…sing-blob - Fastly Config Store value cap corrected from 64 KiB to 8 000 characters per the official Fastly docs (Q6 table + §12.3 Fastly-specific size test). - "Split across [stores.config] ids" workaround reworded as "restructure C into separate typed structs" — the framework does not auto-split one C, and §3.2's no-multi-blob-merge stance forbids it. - Parent Command::Config after_help via the existing tuple variant (#[command(subcommand, after_help = ...)] Config(ConfigCmd)) instead of converting to a struct variant. - Q3 missing-blob behaviour reversed from EdgeError::Internal to EdgeError::ConfigOutOfDate so the response class matches ConfigOutOfDate's "re-run config push" rationale.
Round 18 reversed Q3's missing-blob default from EdgeError::Internal to EdgeError::ConfigOutOfDate and updated the §3.3.3 extractor sketch, but §6.3's narrative bullet and the ConfigStoreError → EdgeError mapping table still showed the old Internal mapping. Round 19 brings §6.3 into line: - The "Key missing from the store" bullet now states EdgeError::ConfigOutOfDate (HTTP 503, Retry-After: 60, "run <app-cli> config push" message) and cites Q3 (d) + §3.3.3. - The mapping table swaps the stale "Internal (none) → Internal" row for "missing key (Ok(None) from get) → ConfigOutOfDate" with a Notes column noting the round-18 M-2 reversal.
Two findings from round 20: - §5.2.1's extractor-consumption sketch (illustrating RequestContext::config_store_default_binding) was still mapping ConfigStore::get(key) == Ok(None) to EdgeError::internal. Rounds 18/19 had updated §3.3.3 + §6.3 to ConfigOutOfDate; this secondary sketch is now aligned. §12.1's missing-key test now pattern-matches the variant and asserts 503 + Retry-After: 60. - Spin Cloud CLI surface was overstated. Per the Fermyon Cloud command reference, "spin cloud key-value list" enumerates STORES (not keys), "delete" removes a STORE (not individual keys), and "set" is the only per-key operation. §8.3 reworded; the §10.x cleanup recipe replaced (delete-per-leaf) with three real options: leave orphans, recreate the store, or use a separately documented API/dashboard path.
Round-21 reviewer probed serde_canonical_json v1.0.0 (the previously surveyed Q1 (a) external crate) and found it errors with "Floating point numbers are forbidden" on any finite float. §4.2 explicitly requires finite-f64 support via ryu, so (a) cannot implement the spec unchanged. - Q1 marked "resolved to (b)": the v1 canonicaliser is the in-tree hand-rolled walker at crates/edgezero-core/src/canonical_form.rs, depending only on serde_json + ryu + sha2 (all in-tree). - §4.2 "Stability across implementations" block rewritten — no external crate name + version pin to record; the only placeholder the spec still leaves is the 64-character SHA hex in canonical_form_pin_v1. - §13.1 acceptance gate simplified — drops the "external dep OR in-tree module" branching, requires the in-tree module to exist, fails loud if a future PR tries to revive serde_canonical_json.
…regroup Five findings, all of which would have leaked into the implementation plan as bugs or unresolved questions: - §10.3 manifest-schema scope corrected. The §5.2 hyphen-rejection rule IS a hard-cutoff manifest-schema change; §10.3 now carves this out explicitly, names the renamed sites operators must touch, and §10.2 gains a grep -RnE recipe for auditing in-tree trees + operator manifests. - §3.3.3 extractor sketch's deserialize call corrected from the non-compiling serde_path_to_error::Deserializer::new(data, &mut track) form to use serde::de::IntoDeserializer + serde_path_to_error::deserialize, matching §4.3. - Q9 audit log resolved to (c) — out of v1 scope. §8.2 / §12.2 / §13 carried no contract for it; rather than under-specify, drop from v1 and treat as a follow-up. - §13 commit phasing regrouped for bisect-friendliness. The old phasing split --key, diff, and read-back across late commits. New phasing puts ConfigStoreBinding + manifest-charset with the extractor, --key with push, inline diff with push; each commit annotated as a complete slice. - §8.4 dangling references fixed (the diff format docs live in §8.1.1 / §8.1.2 / §8.1.3).
Two findings from round 23: - §13 phasing rewritten to honor §10.1's no-platform-bridge rule. Round 22 marked every commit as a "complete slice", but an intermediate commit with new runtime + old writer (or vice versa) cannot run end-to-end, and the §10.2.1 grep gate fails in any commit that moves the extractor without migrating app-demo handlers in the same commit. Phasing collapsed from 7 commits into 5, with explicit per-commit annotations: complete slice / cutover slice / pre-cutover infrastructure / post-cutover additive. The extractor + push rewrite + app-demo migration + scaffold templates + grep gate now land in one cutover commit (Commit C). - §12.6.1 added: per-variant tests for the kind string, field_path presence/absence, Retry-After: 60 on ConfigOutOfDate only, and HTTP status codes. Round-22 test plan covered only the ConfigOutOfDate body; §6.3.1's body-shape change touches every EdgeError variant, so a future refactor needs guard-rails on the other seven too.
Reviewer cleared the spec for plan authoring with no design blockers; only three doc-hygiene items remained: - §13 "land Stages A-C now, do D-F later" → "land Commits A-B now, do C-E later" (matches the round-23 five-commit phasing). - §13 "PR cannot land without ALL six" → "ALL five (Commits A-E)". - §1 round-2 changelog reference "one PR, seven commits" updated to "five commits (round-23 collapse from the earlier seven-commit plan)" so readers walking the v1 changelog don't trip over the stale number. - Status header was "v1 — Draft, pending first review"; updated to "v1 — Plan-ready (twenty-three reviewer passes complete; reviewer cleared for plan authoring at round 24)".
Five-phase plan derived from spec docs/superpowers/specs/2026-06-16-blob-app-config.md §13: - Phase A — canonical form + envelope + non-finite-float rejection (pre-cutover infra, complete slice). - Phase B — manifest charset + ConfigStoreBinding + EnvConfig::store_key + EdgeError::ConfigOutOfDate + read trait + per-adapter read impls (pre-cutover infra, complete slice). - Phase C — THE ATOMIC CUTOVER: AppConfig<C> extractor + per-adapter writer wiring + app-demo migration + scaffold templates + three CI gates (single commit per §10.1). - Phase D — config diff command (post-cutover additive). - Phase E — migration guide + smoke scripts + READMEs. Plan incorporates rounds 25 + 26 of pre-execution review: - Round 25 fixed CLI return types (AppConfig::named / from_store return C, not Self), removed pipe-tail masking from 29 cargo commands, pinned Spin Cloud writer shape, rewrote B2 against the actual EdgeError surface (inner/message/status, not source/Display). - Round 26 mirrored Adapter::read_config_entry to the existing push_config_entries signature parameter-for-parameter (including manifest_root + AdapterPushContext), moved envelope construction from per-adapter writer code to the CLI (adapters keep their existing (String, String) writer surface unchanged), and notes that Spin Cloud's existing write_batch + diagnostics stay intact.
Plan author discovered that running scaffold_render against the §13 Commit C cutover would fail if cli/src/main.rs.hbs already declared TypedConfigCmd::Diff — because ConfigDiffArgs + run_config_diff_typed land in Commit D. §10.2.2's cli/src/main.rs.hbs block now carries an "Implementation phasing note" stating that Commit C ships the template's TypedConfigCmd enum with Push + Validate only, and Commit D adds the Diff variant + dispatch in the same commit that ships the diff implementation. The bundled binary's stub variants stay stable across both commits; only the scaffold template's TypedConfigCmd grows the new variant in Commit D. Round-26 changelog entry added with the same rationale.
Round-26 B12/B13 changed Adapter::read_config_entry to mirror the existing push_config_entries parameter list (manifest_root, adapter_manifest_path, component_selector, store, key, push_ctx). B13's per-adapter impl samples were updated to the new shape, but the caller sketch in C4 Step 2 still constructed a (now-removed) ReadConfigContext struct. Replaced with positional-argument calls to read_config_entry / read_config_entry_local matching B12's trait signature, reusing the same locals (manifest_root, adapter_manifest_path, component_selector, store, key, push_ctx) the writer call uses. Selection between remote and local is the args.local branch.
…tch parity Seven findings from round 27: - Spec ReadConfigEntry::Present(Vec<u8>) → Present(String). All §9.x per-adapter prose was already string-based; spec enum aligned. Plus the §9.0 "Present(bytes)" callout reworded to "Present(body)". - Plan D2 Step 2 + 3 rewritten with explicit per-variant DiffOutcome handling (NoChanges / DiffPresent / RemoteAbsent / Unsupported) and a five-row --exit-code semantics table covering each outcome plus the error class. apply_exit_code helper sketched. - Plan B13 Step 4 expanded to mirror Spin's four-branch write dispatch (--local SQLite-direct, deploy-command Fermyon Cloud detection, runtime-config Redis/Azure/Unknown rejections, default SQLite). Read-side now uses runtime_config::read + KeyValueBackend matching, symmetric with the writer at cli.rs:514. - Plan D1 scope clarified to "ConfigDiffArgs clap struct (bundled-CLI exports only)" — the bundled binary still uses ConfigCmdStubArgs; D2 Step 4 wires the scaffold template TypedConfigCmd::Diff variant. - Plan timestamp helper: chrono = "0.4" already in workspace deps; generated_at_rfc3339() now uses chrono::Utc::now().to_rfc3339_opts. Tech Stack updated to note edgezero-cli is a new chrono consumer. - Spec §10.2 high-level handlers.rs.hbs bullet corrected: import is a COMMENTED sample, not active. §10.2.2 had the right detail; the one-line summary at §10.2 disagreed. - Plan coverage table §12.9 row updated to C6 (Push + Validate) + D2 (Diff per round-26 phasing carve-out).
…ape pin Six findings from round 28: - C1 extractor uses map_err(EdgeError::from) for ConfigStore::get errors so the existing From<ConfigStoreError> impl at error.rs:148 fires (Unavailable→ServiceUnavailable, InvalidKey→BadRequest, Internal→Internal per spec §6.3). Earlier sketch wrapped everything as EdgeError::internal — three regression tests added. - B13 Spin read-back propagates runtime_config::read errors AND calls verify_label_declared, matching the writer at cli.rs:629 + :653. Earlier .ok() form silently swallowed parse errors and fell through to the default SQLite branch, letting `config diff` succeed on trees where the writer would have failed. - Spec §4.2 pins JSON string escaping byte-identical to serde_json::to_string. Adds the full table (named escapes for " \\ \\n \\r \\t \\b \\f; \\u00XX for other control chars). Plan A2's write_string updated to match. Pin-test fixture (A3) extended to include tab/quote+backslash/newline + a U+0001 control char so every escape branch is exercised. - B10 trybuild fixture list extended with the two missing authoritative §3.3.1.4 rules: sibling-exists-but-not-store_ref, and sibling-not-String. Plus rename / container-rename_all / non-String fixtures spelled out so coverage maps 1:1 to the table. - C4 Step 1 now spells out the loader swap explicitly: load_app_config_with_options runs Validate::validate internally, so just replacing the post-load validate call leaves double-validation with secret-bearing fields validated as KEY NAMES (the exact failure mode §3.3.8 forbids). Step 1 instructs switching the loader to deserialize_app_config_with_options first. - B11 Files block lists the typed_secret_checks update at crates/edgezero-cli/src/config.rs:671 so the implementer doesn't hit a non-exhaustive-match surprise when KeyInNamedStore is added.
…ests
Five findings from round 29:
- B13 Spin SQLite read path now reuses push_sqlite::resolve_sqlite_path
from the writer (push_sqlite.rs:97). Earlier sketch used
spin_manifest_dir.join(p), but the writer anchors relative paths
against the runtime-config FILE's directory — not the Spin manifest
directory. The mismatch broke diff / skip-on-equal when
--runtime-config lived outside the Spin manifest dir. Three new
Spin fixtures: relative path resolves against runtime-config dir,
default path falls back to manifest dir, absolute path honoured.
- B7 Step 5 adds per-adapter request-context-builder tests proving
EnvConfig::store_key("config", id) actually packs into
ConfigStoreBinding.default_key on every adapter. The B7 Step 3
EnvConfig::store_key unit test passes but doesn't catch a
per-adapter regression. Coverage row §12.7 updated to enumerate
the three layers (B7 Step 3 unit, B7 Step 5 per-adapter,
E2 smoke).
- B11 Spin validate_typed_secrets sketch switched from HashSet<String>
to HashMap<String, &str> so the collision error names BOTH
conflicting field names per spec §12.16 (current cli.rs:378
HashSet only knows ONE field name). Three explicit §12.16
fixtures added: hyphen-in-value rejection, two-field collision
on lowercased Spin variable, non-Spin-adapter exemption.
- C7 Cargo.toml block adds proc-macro2 = { version = "1", features =
["span-locations"] } so the syn-based nested-AppConfig helper can
print <file>:<line>:<field> violation lines. Without the feature,
Span::start() returns line: 0, column: 0 and the gate's promised
output is unanchored.
- Spec §4.2 canonical_form_pin_v1 fixture expanded to match the
spec's "covers quote, backslash, newline, U+0001" prose. Plan
already had the expanded fixture; spec now matches.
…ter type names Three findings from round 30: - B5 Step 5 + B7 Step 4 + B7 Step 5 now correctly point at crates/edgezero-adapter-axum/src/dev_server.rs (NOT request.rs) for Axum's build_config_registry. The current Axum builder at dev_server.rs:444 takes Option<StoreMetadata> with NO EnvConfig parameter; B7 Step 4 changes the signature to add `env: &EnvConfig` and updates the single call site at dev_server.rs:370 to pass the in-scope EnvConfig. The other three adapters' builders (cloudflare/request.rs:362, fastly/request.rs:382, spin/request.rs:276) already take env. B7 Step 5's per-adapter default_key tests likewise point at dev_server.rs's `mod tests` for Axum. - E2 expanded with explicit per-adapter §12.7 runtime composition smoke. Loops axum / cloudflare / fastly / spin-local: pushes default + staging blobs, boots runtime with EDGEZERO__STORES__CONFIG__APP_CONFIG__KEY set, asserts staging comes back; unsets, asserts default. Spin Cloud Unsupported branch covered as a separate block (skippable via env var) that asserts config diff errors with the §8.3 message and config push --yes succeeds. Three-layer §12.7 coverage table added: EnvConfig::store_key unit + per-adapter registry-builder unit + E2 runtime composition smoke. - B11 §12.16 test snippets renamed from SpinAdapter / AxumAdapter to the real types SpinCliAdapter / AxumCliAdapter (spin/cli.rs:134, axum/cli.rs:109). Mechanical rename, no semantic change.
Six findings from round 31:
- D2 run_config_diff_typed signature changed from
Result<(), String> to Result<DiffExit, String>. DiffExit
{ code: i32 } carries the success-branch exit code (0 / 1 /
2-for-Unsupported). Scaffold cli/src/main.rs.hbs Diff arm now
matches Ok(DiffExit { code }) and calls process::exit(code)
before the generic fall-through. Generic fall-through bumped
from exit(1) to exit(2) so diff errors satisfy Q10's "errors
always >=2" rule. Without this, the bundled / generated main
at main.rs:27 + main.rs.hbs:83 exits 1 for every Err and the
required >=2 error tests would fail.
- E2 smoke loop reshaped from `for adapter in axum cloudflare
fastly spin-local` to a (adapter, extra-push-flags) array:
axum (no flag), cloudflare --local, fastly --local, spin
--local. spin-local is NOT a registered adapter (spin/cli.rs
:174 only registers "spin"); cloudflare/fastly/spin need
--local to seed the local emulator state per args.rs:190.
Axum's push is always local — no flag needed.
- Spec §5.2.1 stale "already calling env.store_name('config',
id)" qualifier dropped for Axum: dev_server.rs:444's builder
takes no EnvConfig today; this round wires it through.
Cloudflare/Fastly/Spin builders already take env. Two
paragraphs updated.
- B13 Spin read-back KeyValueBackend::Unknown match arm uses
the real `type_name` field (not the fictitious `type_str`).
cli.rs:640's writer code matches type_name; the plan would
not compile as-written.
- Spec §12.16 narrative SpinAdapter -> SpinCliAdapter to match
cli.rs:134's real struct.
- Spec §5.2.1 markdown table at line 3813 had a `|b| b.handle`
closure literal whose pipes were interpreted as column
separators, splitting `Config::default()`'s row into three
cells. Closure rewritten as "unwraps binding.handle"; prettier
now passes (plan + spec both clean).
…, inline diff
Three findings from round 32:
- C4 ReadConfigEntry::Present branch now (a) maps serde_json::Error
to String via map_err — the function returns Result<_, String>,
so the bare `?` couldn't auto-convert — AND (b) calls
remote_envelope.verify() BEFORE the sha-compare so a corrupted
remote blob whose stored sha happens to match the local sha
doesn't silently skip the write. D2 already had this shape; C4
now matches.
- DiffExit propagation completed: spec §3.2.2 sketch updated from
Result<(), String> to Result<DiffExit, String> with a phasing
note. Spec §10.2.2's scaffold main.rs.hbs dispatch block
rewritten to match Ok(DiffExit { code }) and call exit(code)
before the fall-through; fall-through bumped from exit(1) to
exit(2). D2 Step 4 stale `?`-suffix replaced with the same
pattern-match. New D2 Step 5 adds the lib.rs re-exports
(pub mod diff; pub use config::{run_config_diff_typed, DiffExit};
pub use args::ConfigDiffArgs) so the scaffold's
`use edgezero_cli::{run_config_diff_typed, DiffExit};` line
compiles.
- C4 Step 5 defines print_unified_diff_inline directly in
config.rs so the cutover commit has a working inline renderer.
Phase D's diff.rs ships the full --format dispatch; the inline
helper is the unified-only subset push uses pre-write. Includes
the BTreeMap-based leaf walk + dotted-path sort per spec §8.1.1.
…ting
Five findings from round 33:
- C4 Step 2 MissingKey / MissingStore branches now render
print_unified_diff_inline against an empty remote so the operator
sees what's about to be written on first push / dry-run. Spec
§8.1's missing-remote semantic ("every leaf is added") explicitly
enforced; MissingStore adds the provisioning hint. Earlier draft
just commented and proceeded to consent.
- C4 Step 5 print_unified_diff_inline doc-comment narrowed: the
helper emits PER-LEAF (added)/(removed) lines and does NOT roll
up whole-subtree transitions into a single parent-path block per
spec §8.1.1. Phase D's diff.rs ships the subtree-roll-up form
behind --format unified; the cutover commit's inline helper is
the per-leaf flat fallback. C4 Step 6 tests assert per-leaf
behaviour; spec §8.1.1's subtree-roll-up assertion lives in
Phase D's tests.
- E2 smoke TOML expanded from one-line `greeting = "..."` to a
full AppDemoConfig fixture (api_token, feature.new_checkout,
greeting, service.timeout_ms, vault). The current AppDemoConfig
at examples/app-demo/.../config.rs:19 has #[serde(deny_unknown_
fields)] AND #[validate(nested)] on feature/service, so a partial
TOML would fail at push time. The smoke now varies only greeting.
- Spec §3.2.2 adds the actual `pub struct DiffExit { pub code: i32 }`
declaration block inline; earlier note pointed at "§3.2.2's
DiffExit declaration block" but no such block existed.
- D2 Step 5 lib.rs additions now explicitly mirror the existing
#[cfg(feature = "cli")] gating at lib.rs:22-48. Without the
gate, `mod diff` and the new re-exports would pull into every
default-features = false consumer (including wasm builds);
prose now spells out the gating + the alphabetical merge into
the existing pub use config::{...} block.
…impls
Three findings from round 34 + a stream-discipline clarification
prompted by the user's println vs logging question:
- C4 Step 5 print_unified_diff_inline now uses a short_ref(sha)
helper that returns the input unchanged when it's <= 8 bytes.
Earlier &sha[..8] slice panicked on the "(none)" sentinel that
round-33 H-1 passes for MissingKey / MissingStore (6 bytes),
turning the all-leaves-added first-push render into a crash.
- C1 first_violating_field walks ValidationErrorsKind::
{Field, Struct, List} recursively and emits dotted paths
(e.g. "service.timeout_ms"). Earlier impl only sorted top-
level keys, which collapsed every nested-validator failure
to its parent key and made EdgeError::ConfigOutOfDate.
field_path useless for app-demo's #[validate(nested)] cases
on FeatureConfig + ServiceConfig.
- C3 Step 1 + D1 Step 1 explicitly extend the manual Default
impls. ConfigPushArgs has a hand-rolled Default at args.rs:
226 (the #[non_exhaustive] attribute disables the auto-
derived one); the three new fields (key, yes, no_diff) need
to be added in alphabetical order. ConfigDiffArgs gets the
same manual impl mirroring the ConfigPushArgs pattern so
§12.11 parser-roundtrip tests can use struct-update syntax.
- Stream discipline (round-34 user prompt): diff CONTENT goes
to stdout via println! (so operators can pipe --format json
to jq); informational MESSAGES (# no changes, # no remote,
the provisioning hint) go to stderr via eprintln!. We do NOT
use log::* — env_logger's [INFO ...] prefixes + timestamps
would corrupt both the TTY render and downstream parsers.
C4 Step 5 doc-comment now spells the convention out; five
informational println! sites converted to eprintln!.
Round-35 reviewer asked "are we using a library for diff" and followed up with an explicit ask to switch to a diff library and normalise (format) JSON values before comparing. The C4 Step 5 helper was hand-rolling a per-leaf walker + custom render — about 80 LOC the standard library can do better. - Add `similar = "2"` as a fourth workspace dep in Task A1 + the Tech Stack line. - Spec §8.1.1 rewritten from the custom per-leaf format to standard text-based unified diff (git-style `@@ -1,9 +1,10 @@` hunks), which is what operators expect from a `diff` command. - C4 Step 5: replace `print_unified_diff_inline` + `walk_leaves` with `similar::TextDiff::from_lines` over `render_for_diff` (recursive key sort + serde_json pretty-print). Renderer is now ~30 LOC; the round-33 "subtree narrowing" doc-comment goes away because text diff handles subtrees naturally as multi-line +/- blocks. - D2 Step 1 clarifies dispatch: `unified` re-uses the C4 Step 5 helper directly (no duplication), `structured`/`json` keep a `walk_leaves` helper inside `diff.rs` for programmatic Vec<DiffEntry> output. Same key-sort ordering as `render_for_diff` so empty-diff outputs match across formats.
H-1 (compile-blocker): validator 0.20's `errors()` returns
HashMap<Cow<'static, str>, _>, not <&'static str, _>. The C1
recursive `first_violating_field` helper used `Vec<&'static
str>` via `.copied()` — `Cow` isn't `Copy`, so the sketch
wouldn't compile. Fixed three sites:
- plan C1 helper: `Vec<&str>` via `.keys().map(|k| k.as_ref())`
- plan B9 comment on `errors_mut()`
- spec §3.3.8 comment on `errors_mut()`
The `HashMap::get(key)` call still works since `Cow<'static,
str>: Borrow<str>` (and `remove(field.name)` likewise — that's
a `&'static str` which coerces to `&Q` where `Q = str`).
M-1: `config push` was reading remote ONCE (for diff render)
then proceeding straight to writer after consent — leaving the
final skip-on-equal decision against a pre-consent snapshot.
Spec §8.1.4 + §8.2 are explicit that skip-on-equal re-fetches
right before the write. Added C4 Step 3.5 between consent and
writer that re-reads (only when first read was Present + not
dry-run), re-checks the sha, and:
- exits with "concurrent push reached the same state" if a
parallel push beat us to the same data
- logs a "remote changed between diff and write" warning if
the remote moved to a different non-local sha (consent
was for "apply local" — we don't re-prompt and loop)
- falls through if remote disappeared (Present → MissingKey)
Plus three new tests: same-data-concurrent, different-data-
concurrent, dry-run-skips-second-read.
L-1: Both files failed prettier when run from repo root (the
docs config resolves the same either way, but a malformed
markdown block at spec §4.2's float-rejection section had
gotten progressively mangled by previous --write rounds —
backticks running into words, numbered-list indent collapsing
into outdent, etc.). Restructured the block into clean prose
paragraphs with proper 4-space sub-bullet indent. Prettier
--check now passes.
H-1 (compile-blocker): C4 Step 3.5's re-fetch sketch referenced
`remote_envelope.sha256`, but `remote_envelope` was declared
INSIDE the `ReadConfigEntry::Present` match arm at Step 2 — out
of scope by the time Step 3.5 runs. Hoisted the first-read SHA
into an outer `Option<String> approved_remote_sha` populated by
the `Present` arm. Step 3.5's "remote changed between diff and
write" comparison now reads `approved_remote_sha` via
`if let Some(ref approved) = ...`. Also switched the match to
`match &remote` so the variable survives for Step 3.5's
`matches!(remote, ReadConfigEntry::Present(_))` gate without
move-of-borrowed-content issues.
M-1: C4 Step 6 test still asserted the dotted-path
`(added)/(removed)` format. That format was removed in round-35
when the renderer switched to `similar` over normalised JSON.
Rewrote the test to assert the standard unified-diff shape:
`---`, `+++`, `@@`, JSON pretty-printed `±` lines with 2-space
indent, recursive key-sort ordering (feature < greeting <
service), and a multi-line `+` block for an added subtree.
Plus introduced `print_unified_diff_to_writer` as a writer-
taking sibling so tests can capture into a `Vec<u8>`.
M-2: Spin read-back dispatch table had unescaped `|` characters
inside row 3's Rust OR-pattern
`Some(Redis { .. } | AzureCosmos | Unknown)`. GFM treated those
as column separators, fanning out the header row to 5 columns
and splitting the trigger across cells. Restructured the
four-branch dispatch as a numbered bullet list with
**Trigger:** / **Read-side behaviour:** sub-bullets — more
readable AND no pipes for markdown to misinterpret.
L-1: Dropped unused `ChangeTag` from
`use similar::{ChangeTag, TextDiff};`. The unified-diff path
goes through `TextDiff::unified_diff()` builder which handles
per-change classification internally; `ChangeTag` would be
needed only if we walked individual edits manually. Clippy
`-D warnings` would otherwise reject.
H-1 (behaviour gap): the round-36 re-fetch only fired when the
first read was `Present`. That left the MissingKey/MissingStore
→ Present transition uncovered — a concurrent operator could
push between our first read and our consent gate, and we'd
blindly overwrite the just-pushed blob with no skip-on-equal
protection. Spec §8.1.4 + §8.2 are explicit that the skip-on-
equal CHECK runs right before the write for every read-capable
adapter. Widened the gate from
`matches!(remote, ReadConfigEntry::Present(_))`
to
`!matches!(remote, ReadConfigEntry::Unsupported(_))`.
Behaviour after the change:
- first Present, second Present, same SHA: skip (unchanged).
- first Present, second Present, different SHA: warn
"remote changed between diff and write (was A now B)"
and proceed (unchanged).
- first Present, second MissingKey/MissingStore: NEW —
warn "remote was removed between diff and write" and
fall through (creating not overwriting).
- first MissingKey/MissingStore, second Present, == local:
NEW — skip with "concurrent push reached the same state".
- first MissingKey/MissingStore, second Present, != local:
NEW — warn "remote changed between diff and write
(was (none) now <sha>)" and proceed.
- first MissingKey/MissingStore, second same: fall through.
- first Unsupported: skip the whole second read.
Display uses `approved_remote_sha.as_deref().map_or("(none)",
short_ref)` so the "(none)" sentinel covers the missing-first
path. Added three new tests covering the missing→present
transitions and the present→missing transition.
L-1: the spec §8.1.1 example showed `"nested": { "alpha": 1,
"beta": 2 }` on one line, but the spec text says
`render_for_diff` uses `serde_json::to_string_pretty` — which
always emits multi-line objects with 2-space indent. The
example also showed `@@ -1,9 +1,10 @@` line counts that
wouldn't match the pretty-printed reality. Rewrote the example
with the actual multi-line nested block (4 `+` lines for the
new subtree) and the correct `@@ -1,11 +1,15 @@` hunk header.
L-1: Step 2's deserialize-skips-validate test snippet derived
`AppConfigMeta` via `#[derive(Deserialize, AppConfigMeta)]`,
but the real derive macro is `#[derive(AppConfig)]` (which
EMITS the `AppConfigMeta` impl). Either form would also be a
cycle anyway: this test lives in `edgezero-core`, and the
derive macro lives in `edgezero-macros` which depends on
`edgezero-core`. Promoted the hand-rolled `impl AppConfigMeta
for Fixture { const SECRET_FIELDS = &[]; }` to be the primary
snippet, with a short note explaining the cycle. The derive
coverage stays in `edgezero-macros`'s own tests and the
app-demo's tests.
L-2: Plan snippets referenced `http::header::CONTENT_TYPE` /
`http::header::RETRY_AFTER` directly, while project style at
`crates/edgezero-core/src/error.rs:8` imports through
`crate::http::{header::CONTENT_TYPE, ...}`. Rewrote the
snippets to use bare `CONTENT_TYPE` / `RETRY_AFTER` (assumed
imported via `crate::http::header`), with an inline note
steering implementation to extend the existing `use` line
rather than pulling from the raw `http` crate.
L-3: Removed the trailing "Plan complete... Which approach?"
block. That was the writing-plans skill's planning-mode offer
left over from initial authoring; the plan is now the saved
execution artefact and the block read like we were waiting
for selection. Replaced with one sentence pointing at
"Execute Phase A first" per spec §13's phase ordering.
Phase A of the blob app-config rewrite per docs/superpowers/specs/2026-06-16-blob-app-config.md §4.1-§4.2: - crates/edgezero-core/src/canonical_form.rs: hand-rolled SHA canonicaliser over serde_json::Value. Sorts object keys by UTF-8 byte order, renders finite floats via ryu's shortest round-trippable form, emits verbatim UTF-8 strings (no NFC normalisation). Q1 resolution per spec round 21: in-tree only, no external canonicaliser crate. - crates/edgezero-core/src/blob_envelope.rs: BlobEnvelope type wrapping data + sha256 + version + generated_at. verify() recomputes the sha and rejects mismatch or unknown version. - crates/edgezero-core/src/app_config.rs: new AppConfigError:: InvalidValue variant; loader (TOML walk + env overlay) rejects non-finite f64 values before serialisation, so serde_json never coerces them to JSON null. - canonical_form_pin_v1 test in crates/edgezero-core/tests/ pins the v1 byte format against a fixed fixture. - deserialize_app_config[_with_options] entry points expose the deserialise-only path for Phase C's push/diff routing. No in-tree caller exercises any of this yet; Phase B wires it into infrastructure (extractor, read trait, etc.) and Phase C ships the cutover.
Phase B of the blob app-config rewrite per
docs/superpowers/specs/2026-06-16-blob-app-config.md.
- §5.2 manifest store-id charset tightened from [A-Za-z0-9_-]
to [A-Za-z0-9_]+ so the EDGEZERO__STORES__<KIND>__<ID>__KEY
override is POSIX-shell-exportable. Hyphens rejected at
manifest validation with an actionable error.
- §5.2.1 ConfigStoreBinding { handle, default_key } replaces
ConfigStoreHandle as the ConfigRegistry value type. Adapters
build the binding via EnvConfig::store_key("config", id).
Config::default() still returns ConfigStoreHandle (unwraps
binding.handle); new default_binding() accessor returns the
binding for the typed extractor.
- §6.3.1 EdgeError gains the ConfigOutOfDate variant + two
constructors (config_out_of_date / config_out_of_date_from_serde).
Response body adds a stable `kind` field on every variant;
Retry-After: 60 fires only on ConfigOutOfDate.
- §3.3.1 SecretKind::KeyInNamedStore { store_ref_field } added;
the #[derive(AppConfig)] macro accepts #[secret(store_ref =
"field")], rejects #[serde(skip_serializing / skip_serializing_if
/ flatten)] on every field, and emits impl AppConfigRoot for C.
- §3.3.8 validate_excluding_secrets wrapper added.
- §9.0 Adapter::read_config_entry + ReadConfigEntry enum
(Present / MissingKey / MissingStore / Unsupported). All four
adapters implement read + read_local; Spin Cloud returns
Unsupported per round-7 Fermyon-CLI-surface check.
No in-tree caller exercises the new types yet; Phase C wires
them into the extractor + push rewrite + app-demo migration.
aram356
force-pushed
the
feature/extensible-cli
branch
from
524cb98 to
5d72674
Compare
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
3 participants
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.
Epic: #268
Summary
Turns
edgezero-cliinto an extensible library, rewrites the manifest store schema and runtime to a multi-store model, addsauth/provision/config validate/config pushcommands, replaces thedevsubcommand with a contributor-onlydemo, and updatesapp-demoto exercise everything across axum / cloudflare / fastly / spin. Plus two integrations completed in-branch: Spin 6.0 (WASI Preview 2) andchore/strict-clippy(PR #257).Delivered as one PR, eight sequential stages (one per sub-issue of #268), with additional hardening from multi-round self-review and two integration cycles on top.
[stores.*]fields become hard load errors.chore/strict-clippy(PR Enable strict clippy with documented allow-list and defensive-coding pass #257) —origin/chore/strict-clippyhas been merged into this branch (merge commit611064e) so the diff againstchore/strict-clippyis the actual feature delta. Base retargets tomainwhen Enable strict clippy with documented allow-list and defensive-coding pass #257 merges.Stages (sub-issues of #268)
config validatecommandauthcommand +CommandRunnerinfrastructureprovisioncommandconfig pushcommandapp-demointegration polish + docs auditIn-branch integrations on top of the eight stages
Spin 6.0 (WASI Preview 2) —
487ac5fspin-sdk = "6"workspace + app-demo + generator defaultwasm32-wasip1→wasm32-wasip2for the Spin adapter only; Fastly stays on wasip1. CI matrix entry + CLAUDE.md + docs + scripts + scaffold template + app-demo manifests all updated.IncomingRequest→Request;req.into_parts()+IncomingBodyExt::bytes(); deleted the 10-armMethodenum match (Methodis now a re-export ofhttp::Method);Request::builder()…build()→.body(FullBody::new(Bytes::from(...)))?; asyncStore::open/get/set/delete/exists/get_keys; asyncvariables::get;#[http_component]→#[http_service].Response<FullBody<Bytes>>; contract tests usehttp_body_util::BodyExt::collect().chore/strict-clippyintegration —611064eorigin/chore/strict-clippyso the workspace-wide strict-clippy gate (pedantic warn + restriction deny, 13-item allow-list) sits underneath everything in this PR.adapter-wasm-clippyCI matrix added in Enable strict clippy with documented allow-list and defensive-coding pass #257; switched its spin entry from wasip1 → wasip2 to match the Spin 6 migration.Strict-clippy pass on wasm32 adapter targets —
910739dadapter-wasm-clippymatrix to green: cloudflare (~44 errors), fastly (~14), spin (~115).#[allow]sprinkles):pub use mod::Foo→pub mod foo;absolute_paths→useimports;min_ident_chars→ renames (|e|→|err|,|c|→char::is_control);#[inline]+# Errorson every public fn;arbitrary_source_item_orderingreordering;tests_outside_test_module+expect_usedin tests fixed by wrapping in#[cfg(test)] mod tests { ... };used_underscore_items→const _: fn() = assert_provider_impl::<T>;;let_underscore_must_use→drop(init_logger()); type aliasSpinFullResponse = Response<FullBody<Bytes>>for the recurring complex type;HeaderValue::from_static("spin")to killexpect_usedinproxy.rs;.saturating_add()forarithmetic_side_effects.Self-review followups closed in this PR
Two close-out commits address findings from the post-merge self-reviews:
16898f8— Store id validation (rejects blank / whitespace / control / duplicate logical ids withstore_id_blank/store_id_duplicateerrors); docs ESLint ignore for.vitepress/.tempso the docs lint gate is build-state-independent; removed the misleading singularStoreDeclaration::config_store_name(&self, _adapter)helper.4928cdc— Spin wasm contract CI fix (tests insert*Registry::single_id(...)instead of bare handles so they match the dispatch boundary'ssynthesise_store_registries);generated_project_buildsnow invokes the generated typed CLI'sconfig validate --strict(catchesAppConfigdrift the raw validator can't); docs build fix (multi-line inline backticks incli-walkthrough.mdpush bullets converted to fenced shell blocks — VitePress was parsing the leaked<tempfile>/<id>as unterminated HTML tags); provision + push docs now describe the<platform-name>resolution (EDGEZERO__STORES__<KIND>__<ID>__NAME→ logical id) for cloudflare/fastly instead of the wrong "matched by<store_id>" wording; Spin docs corrected on secret translation (config keys translate.→__;#[secret]values are only lowercased — matchesSpinSecretStore::get_bytes+ the cli validator exactly); stale plan doc references updated (app-demo IS in CI; Spin target IS wasip2); axum secret store rustdoc no longer references the removedcargo edgezero devcommand.Design + plan
Committed under
docs/superpowers/(force-added — that path is currently in.gitignore; maintainers can decide whether to formally track it):docs/superpowers/specs/2026-05-19-cli-extensions-design.mddocs/superpowers/plans/2026-05-20-cli-extensions.mdTest plan
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-features -- -D warnings(host strict-clippy)cargo test --workspace --all-targets(1057+ tests pass)cargo check --workspace --all-targets --features "fastly cloudflare spin"cargo check -p edgezero-adapter-spin --target wasm32-wasip2 --features spincargo check -p edgezero-adapter-fastly --target wasm32-wasip1 --features fastlycargo check -p edgezero-adapter-cloudflare --target wasm32-unknown-unknown --features cloudflarecargo clippy -p edgezero-adapter-cloudflare --features cloudflare --target wasm32-unknown-unknown --all-targets -- -D warningscargo clippy -p edgezero-adapter-fastly --features fastly --target wasm32-wasip1 --all-targets -- -D warningscargo clippy -p edgezero-adapter-spin --features spin --target wasm32-wasip2 --all-targets -- -D warningscd examples/app-demo && cargo test --workspace --all-targetscd examples/app-demo && cargo fmt --check && cargo clippy --workspace --all-targets --all-features -- -D warningscargo check -p app-demo-adapter-spin --target wasm32-wasip2cargo test -p edgezero-cli --test generated_project_builds -- --ignored(now also runs the generated typed CLI'sconfig validate --strict)cd docs && npm run lint && npm run format && npm run buildwasmtime run(12/12 — reviewer-verified locally; CI matrix runs the same)