Skip to content

Commit 59b504b

Browse files
committed
Amend plan per review 17 (consent cutover sequencing + secret/consent tests)
- Interim Fastly regression: Task 5 flipped consent to kv_handle_named but Fastly has no KvRegistry until Task 6 -> consent would silently skip on Fastly. Fix: Task 5 migrates the consent TYPE but keeps the call site on kv_handle() (default, = Fastly's swapped consent store); the named-lookup FLIP + behavioral test are atomic in Task 6 Step 5b once all four inject a KvRegistry - Absent-registry policy: builders stay infallible (return RuntimeServices); build a composite over an EMPTY registry so reads error, not silently fall back - Task 6 Step 5c: Fastly named-KV/consent route test (guards the special-case removal) - Task 3 Step 1b: explicit CompositeSecretStore test (named id, unknown-id error, create/delete StoreId-preserving delegation) - flat CF/Spin secret namespaces mean route tests don't prove store-id binding - Task 5 hygiene: no longer edits/commits Fastly (moved to Task 6)
1 parent eb787f8 commit 59b504b

1 file changed

Lines changed: 45 additions & 13 deletions

File tree

docs/superpowers/plans/2026-07-02-edgezero-store-registry-migration.md

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -251,9 +251,38 @@ fn composite_config_reads_named_store_and_writes_delegate() {
251251
}
252252
```
253253

254-
- [ ] **Step 2: Run to verify it fails**
254+
- [ ] **Step 1b: Write the failing SECRET-composite test too** (not just config — Cloudflare/Spin secrets are a **flat** namespace, so route tests won't prove store-id binding; this unit test does). Mirror the config test for `CompositeSecretStore`:
255+
```rust
256+
#[test]
257+
fn composite_secret_reads_named_store_and_writes_delegate() {
258+
// SecretRegistry with default + a non-default `ts_secrets` id.
259+
let reader = secret_registry(&[
260+
("trusted_server_secrets", "API_KEY", b"default-key"),
261+
("ts_secrets", "server-side-key", b"dd-secret"),
262+
], "trusted_server_secrets");
263+
let writer = Arc::new(RecordingSecretWriter::default());
264+
let composite = CompositeSecretStore::new(reader, writer.clone());
265+
// Non-default store resolves.
266+
let v = composite.get_bytes(&StoreName::from("ts_secrets"), "server-side-key").expect("read");
267+
assert_eq!(v, b"dd-secret");
268+
// Unknown store id is a strict error.
269+
assert!(matches!(
270+
composite.get_bytes(&StoreName::from("nope"), "x").expect_err("should error on unknown secret store").current_context(),
271+
PlatformError::SecretStore,
272+
));
273+
// create/delete delegate with the target StoreId preserved.
274+
composite.create(&StoreId::from("ts_secrets"), "new", "val").expect("create delegates");
275+
assert_eq!(
276+
writer.creates.lock().expect("should acquire writer lock").as_slice(),
277+
&[("ts_secrets".to_owned(), "new".to_owned(), "val".to_owned())],
278+
"create must delegate with the SAME StoreId",
279+
);
280+
}
281+
```
255282

256-
Run: `cargo test-fastly composite_config_reads_named_store_and_writes_delegate`
283+
- [ ] **Step 2: Run to verify both fail**
284+
285+
Run: `cargo test-fastly composite_config_reads_named_store_and_writes_delegate composite_secret_reads_named_store_and_writes_delegate`
257286
Expected: FAIL (module does not exist).
258287

259288
- [ ] **Step 3: Implement `composite.rs`**
@@ -371,40 +400,39 @@ cargo test-axum first_party_proxy_reads_s3_secret
371400
```
372401
Expected: FAIL (all three).
373402

374-
- [ ] **Step 2: Build `RuntimeServices` via the composite** in each adapter's `build_runtime_services(ctx: &RequestContext)`. **Extract the whole registry from request extensions**`ctx.request().extensions().get::<ConfigRegistry>().cloned()` / `get::<SecretRegistry>()` — the same way EdgeZero's `Config`/`Secrets` extractors do. Do **not** use `ctx.config_store_default()`/`config_store(id)` (those return a single bound handle and would wire only the default store). Pass the cloned registry as the composite reader (Task 3) and the per-adapter **write-only** impl (`PlatformConfigWriter`/`PlatformSecretWriter`) as the writer. If a registry is absent from extensions, that is a wiring bug (Step 0 / EdgeZero dispatch) — surface it, don't silently fall back.
403+
- [ ] **Step 2: Build `RuntimeServices` via the composite** in each adapter's `build_runtime_services(ctx: &RequestContext)`. **Extract the whole registry from request extensions** — `ctx.request().extensions().get::<ConfigRegistry>().cloned()` / `get::<SecretRegistry>()` — the same way EdgeZero's `Config`/`Secrets` extractors do. Do **not** use `ctx.config_store_default()`/`config_store(id)` (those return a single bound handle and would wire only the default store). Pass the cloned registry as the composite reader (Task 3) and the per-adapter **write-only** impl (`PlatformConfigWriter`/`PlatformSecretWriter`) as the writer. **Absent-registry policy (concrete):** `build_runtime_services` returns `RuntimeServices` (not `Result`) on all adapters, so don't add a fallible signature. Instead, when a registry is absent from extensions, build a composite over an **empty** registry (`StoreRegistry` with no ids) — its strict `named()`/`default()` return `None`, so the composite's `get`/`get_bytes` **error** (`PlatformError`) on first read rather than silently reading a default store. A missing registry thus surfaces as a read error at the call site, not a silent fallback, with no builder signature change.
375404

376405
- [ ] **Step 2b: Non-default coverage on Cloudflare AND Spin (not just Axum).** Cloudflare/Spin platform mappings differ (config = KV-namespace/label backed; secrets = flat namespace), so default-only assertions are insufficient. Add, in each of the Cloudflare and Spin test modules, tests proving **non-default** resolution: a `jwks_store` **config** read and a `ts_secrets` / S3 **secret**-key read resolve through the composite (route tests if cheap, else small `build_runtime_services` + composite-read tests seeding a 2-id registry).
377406

378407
- [ ] **Step 2c: Named-KV resolution — a CORE surface change, not adapter-only (DECIDED — registry-backed KV now).** The core `RuntimeServices` exposes only `kv_store(&self) -> &dyn PlatformKvStore` — a **single** handle — and consumers that have a store id today **drop it**: `publisher.rs:626` does `settings.consent.consent_store.as_deref().map(|_| services.kv_store())`. So adapter-only changes cannot make `consent_store` resolve. This step is cross-cutting:
379408
- **Type decision — resolve named KV as `KvHandle`, and migrate consent onto `KvHandle`.** `KvRegistry::named(id)` yields a `KvHandle` (a wrapper `{ store: Arc<dyn KvStore> }`), **not** a `&dyn PlatformKvStore` — and `ConsentPipelineInput.kv_store` is currently `Option<&dyn PlatformKvStore>` (`consent/mod.rs:89`), so a `KvHandle` does not fit directly. Do **not** wrap; **migrate the consent KV surface to `KvHandle`** (the idiomatic edgezero handle — `RuntimeServices` already exposes `kv_handle()` for the default). Specifically:
380409
- **Core (`platform/types.rs`):** add `RuntimeServices::kv_handle_named(&self, id: &str) -> Option<KvHandle>` (mirroring the existing `kv_handle()`), resolving from a `KvRegistry` carried on `RuntimeServices` (add a `kv_registry` field + builder setter, populated by adapters from `ctx.request().extensions().get::<KvRegistry>()`).
381-
- **Consent (`consent/mod.rs`, `storage/kv_store.rs`, `publisher.rs`):** change `ConsentPipelineInput.kv_store` from `Option<&dyn PlatformKvStore>` to `Option<KvHandle>`; update the consent persistence fns (`load_consent_from_kv`/`save_consent_to_kv`/`delete_consent_from_kv`) to take a `&KvHandle` and use its async methods (they already `block_on`). At the call site (`publisher.rs:626`) pass `settings.consent.consent_store.as_deref().and_then(|id| services.kv_handle_named(id))`.
410+
- **Consent type migration (Task 5) vs behavioral flip (Task 6) — avoid an interim Fastly regression.** Change `ConsentPipelineInput.kv_store` from `Option<&dyn PlatformKvStore>` to `Option<KvHandle>` and update the persistence fns (`load_consent_from_kv`/`save_consent_to_kv`/`delete_consent_from_kv`) to take `&KvHandle` (they already `block_on`) — **in Task 5**. But **do NOT flip the `publisher.rs:626` call site to `kv_handle_named` in Task 5**: Fastly has no `KvRegistry` until Task 6, so `kv_handle_named("consent_store")` would return `None` there and consent persistence would **silently skip on Fastly** between Task 5 and Task 6. Today Fastly consent works because `runtime_services_for_consent_route` (`app.rs:205`) reopens the consent store and **swaps the default** kv via `with_kv_store`. So in **Task 5**, the call site keeps today's behavior: pass `services.kv_handle()` (the default handle — which on Fastly is the swapped consent store; on the others matches current behavior). The **named-lookup flip** — `settings.consent.consent_store.as_deref().and_then(|id| services.kv_handle_named(id))` — and removal of the Fastly swap happen **atomically in Task 6**, once all four adapters (Fastly included) inject a `KvRegistry`. The behavioral test (below) therefore lands in **Task 6**.
382411
- Audit other KV consumers (`ec/*`) for the same "id dropped" pattern; they already use `kv_handle()` so are lower-risk.
383412
- **Adapters — Axum/Cloudflare/Spin here; Fastly in Task 6 (sequencing).** Populate `RuntimeServices.kv_registry` from extensions in `build_runtime_services` for **Axum/Cloudflare/Spin** (they inject registries via EdgeZero `run_app`/`dispatch_with_registries`). **Fastly's** named-KV wiring belongs in **Task 6**, because Fastly only injects registries into extensions in Task 6 (its custom `oneshot`); its active per-request services are built in `app.rs:238` (`build_per_request_services`), not `platform.rs`, and its consent special-casing (`app.rs:205`, `runtime_services_for_consent_route`, used at `app.rs:588/735`) is removed **there** once the `kv_registry` is present. Doing Fastly named-KV in Task 5 would populate from a registry not yet in extensions.
384413
- **Files:** `crates/trusted-server-core/src/platform/types.rs`, `crates/trusted-server-core/src/consent/mod.rs` (`ConsentPipelineInput.kv_store` type), `crates/trusted-server-core/src/storage/kv_store.rs` (consent persistence fns → `&KvHandle`), `crates/trusted-server-core/src/publisher.rs` (call site), `crates/trusted-server-adapter-{fastly,axum,cloudflare,spin}/src/platform.rs`, `crates/trusted-server-adapter-fastly/src/app.rs` (remove consent special-case), adapter test helpers (Step 2d).
385-
- **Test (behavioral, not just resolution) — write it FAILING first.** The migration exists because consent **drops** the id and hits the default store. So the core test that proves the fix is behavioral: with `settings.consent.consent_store = "consent_store"` and a registry holding a **default** KV + a distinct `consent_store` KV, a consent persistence round-trip (load/save/delete) must read/write the **`consent_store`** handle and leave the **default** store **untouched**. Assert against both stores (the target has the entry; the default does not). Add this in `trusted-server-core` consent tests. Also keep the cheaper per-adapter check: `kv_handle_named("consent_store")` resolves and is distinct from the default; unknown id → `None`.
414+
- **Test (Task 5):** the `kv_handle_named` surface resolves — per adapter, `kv_handle_named("consent_store")` returns a handle distinct from the default; unknown id → `None`. (The **behavioral** consent test — that `consent_store` is actually selected and the default is left untouched — lands in **Task 6** with the call-site flip; see Task 6.)
386415

387416
- [ ] **Step 2d: Test-support — registry-populated `RequestContext` helper + migrate existing direct-context tests.** Strict registries make a missing registry a wiring bug, but existing adapter tests call `build_runtime_services(&ctx)` / `build_per_request_services(&ctx)` on **hand-built** `RequestContext`s with no registries inserted (e.g. `adapter-axum/src/app.rs:130`, `adapter-cloudflare/src/app.rs:151,314`, `adapter-spin/src/app.rs:440`, `adapter-cloudflare/src/platform.rs:729`). Those will now fail (composite → `registry.named()``None`). Add a shared test helper (e.g. `test_context_with_registries(config: &[…], kv: &[…], secrets: &[…]) -> RequestContext`) that inserts `ConfigRegistry`/`KvRegistry`/`SecretRegistry` into the context, and **migrate every existing direct-context test** to use it. Enumerate them during the run (`rg 'build_(runtime|per_request)_services'` in adapter test modules).
388417

389418
Also convert the **existing core consent tests** that construct `ConsentPipelineInput` with a `&dyn PlatformKvStore` store — `crates/trusted-server-core/src/consent/mod.rs` has `kv_store: Some(&store)` call sites at ~lines 1450 / 1481 / 1492 — to the new `Option<KvHandle>` shape (build a `KvHandle` over an in-memory `KvStore` test double). These fail to compile the moment `ConsentPipelineInput.kv_store` changes type, so migrate them in the same step (`rg 'kv_store: Some\(' crates/trusted-server-core/src/consent` to find them all).
390419

391-
- [ ] **Step 3: Run to verify pass** — this task touches **core** (`platform/types.rs`, consent, `publisher.rs`) and **Fastly** (`platform.rs`/`app.rs`) as well as Axum/CF/Spin, so run **all four** adapters + wasm checks (per the global "all four green" rule), incl. the non-default config/secret/KV tests from 2b/2c.
420+
- [ ] **Step 3: Run to verify pass** — this task changes **core** (`platform/types.rs`, consent type, `publisher.rs` interim call site) and **Axum/CF/Spin** platform wiring. **Fastly is NOT modified here** (its named-KV wiring + consent flip are Task 6) — but run **all four** anyway to confirm Fastly still compiles/passes with the core changes (the `kv_handle()` interim call site preserves Fastly's swap behavior), per the "all four green" rule.
392421

393422
Run: `cargo test-fastly && cargo test-axum && cargo test-cloudflare && cargo test-spin && cargo check-cloudflare && cargo check-spin`
394423
Expected: PASS.
395424

396-
- [ ] **Step 4: Commit** (include the core + Fastly changes, not just the three adapters)
425+
- [ ] **Step 4: Commit** (core + Axum/CF/Spin — **not** Fastly; Fastly is committed in Task 6)
397426

398427
```bash
399428
git add crates/trusted-server-core/src/platform/types.rs \
400429
crates/trusted-server-core/src/consent/mod.rs \
401430
crates/trusted-server-core/src/storage/kv_store.rs \
402431
crates/trusted-server-core/src/publisher.rs \
403-
crates/trusted-server-adapter-fastly \
404432
crates/trusted-server-adapter-axum \
405433
crates/trusted-server-adapter-cloudflare \
406434
crates/trusted-server-adapter-spin
407-
git commit -m "Wire RuntimeServices via composite + registry-backed named KV across all adapters"
435+
git commit -m "Add named-KV surface + composite reads on Axum/Cloudflare/Spin; migrate consent to KvHandle"
408436
```
409437

410438
---
@@ -446,12 +474,16 @@ Run: `cargo test-fastly build_config_registry_resolves_declared_ids` → Expecte
446474

447475
Run: `cargo test-fastly oneshot_discovery_reads_jwks_via_registry` → Expected: FAIL, then PASS only after Steps 3, 4, **and 4b** (the test reads through `RuntimeServices`, which is composite-backed only after 4b — without 4b the injected registries are unused and the read still hits the old direct store).
448476

449-
- [ ] **Step 6: Fastly suite + parity + commit**
477+
- [ ] **Step 5b: Flip the consent call site to named KV + behavioral test (the atomic cutover — all four adapters now inject a `KvRegistry`).** Now that Fastly (Step 4b) and Axum/CF/Spin (Task 5) all inject a `KvRegistry`, change `publisher.rs:626` from `services.kv_handle()` to `settings.consent.consent_store.as_deref().and_then(|id| services.kv_handle_named(id))`. This is the point where the configured id actually selects the store on **every** adapter (and `runtime_services_for_consent_route`'s swap, removed in 4b, is no longer needed). Add the **behavioral** core test (write it failing first): with `consent_store = "consent_store"` and a registry holding a **default** KV + a distinct `consent_store` KV, a consent round-trip (load/save/delete) reads/writes the **`consent_store`** handle and leaves the **default** store **untouched** (assert against both). Files: `crates/trusted-server-core/src/publisher.rs` (+ core consent test).
478+
479+
- [ ] **Step 5c: Fastly named-KV / consent route test.** With `runtime_services_for_consent_route` removed (4b), add a Fastly test proving `consent_store` resolves via the **injected `KvRegistry`** — a consent-persisting route (or a `build_per_request_services`-level test) writes/reads through the `consent_store` handle, not the default. This guards the special-case removal.
480+
481+
- [ ] **Step 6: Fastly suite + parity + commit** (core consent flip is committed here with the Fastly work, since the flip is only safe once Fastly injects registries)
450482

451-
Run: `cargo test-fastly && cargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --test parity`
483+
Run: `cargo test-fastly && cargo test-axum && cargo test-cloudflare && cargo test-spin && cargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --test parity`
452484
```bash
453-
git add crates/trusted-server-adapter-fastly
454-
git commit -m "Add local Fastly registry builders and inject them into the oneshot dispatch"
485+
git add crates/trusted-server-adapter-fastly crates/trusted-server-core/src/publisher.rs
486+
git commit -m "Inject Fastly registries; flip consent to named KV; remove consent special-casing"
455487
```
456488

457489
---

0 commit comments

Comments
 (0)