Fastly chunked-config GC: reclaim orphaned chunk entries on re-push (last-writer-wins)

.github/workflows/format.yml

@@ -74,10 +74,19 @@ jobs:
         include:
           - adapter: cloudflare
             target: wasm32-unknown-unknown
+            features: cloudflare
           - adapter: fastly
             target: wasm32-wasip1
+            features: fastly
+          # The `cli` feature is a valid wasip1 build combination (the CLI host
+          # code compiled for the guest target); gate it so a dead-code or
+          # portability regression there is caught, not only in the fastly-only build.
+          - adapter: fastly
+            target: wasm32-wasip1
+            features: fastly cli
           - adapter: spin
             target: wasm32-wasip2
+            features: spin
     steps:
       - uses: actions/checkout@v6
 
@@ -110,8 +119,8 @@ jobs:
       - name: Fetch dependencies (locked)
         run: cargo fetch --locked
 
-      - name: Clippy ${{ matrix.adapter }} on ${{ matrix.target }}
-        run: cargo clippy -p edgezero-adapter-${{ matrix.adapter }} --features ${{ matrix.adapter }} --target ${{ matrix.target }} --all-targets -- -D warnings
+      - name: Clippy ${{ matrix.adapter }} (${{ matrix.features }}) on ${{ matrix.target }}
+        run: cargo clippy -p edgezero-adapter-${{ matrix.adapter }} --features "${{ matrix.features }}" --target ${{ matrix.target }} --all-targets -- -D warnings
 
   format-docs:
     runs-on: ubuntu-latest

crates/edgezero-adapter-fastly/Cargo.toml

@@ -49,4 +49,5 @@ walkdir = { workspace = true, optional = true }
 
 [dev-dependencies]
 edgezero-core = { path = "../edgezero-core", features = ["test-utils"] }
+handlebars = { workspace = true }
 tempfile = { workspace = true }

crates/edgezero-adapter-fastly/src/config_store.rs

@@ -1,5 +1,6 @@
 //! Fastly adapter config store: wraps `fastly::ConfigStore`.
 
+use std::cell::Cell;
 #[cfg(test)]
 use std::collections::HashMap;
 
@@ -28,18 +29,6 @@ impl FastlyConfigStore {
         }
     }
 
-    /// Synchronous key lookup used by the chunk-pointer resolver callback.
-    /// Returns `Ok(Some(value))`, `Ok(None)` (missing), or `Err(message)`.
-    fn get_sync(&self, key: &str) -> Result<Option<String>, String> {
-        match &self.inner {
-            FastlyConfigStoreBackend::Fastly(inner) => inner
-                .try_get(key)
-                .map_err(|err| format!("config store lookup failed for `{key}`: {err}")),
-            #[cfg(test)]
-            FastlyConfigStoreBackend::InMemory(data) => Ok(data.get(key).cloned()),
-        }
-    }
-
     /// Open a Fastly Config Store by resource link name.
     ///
     /// Returns an error if the configured store cannot be opened.
@@ -68,30 +57,83 @@ impl ConfigStore for FastlyConfigStore {
         let Some(value) = root_value else {
             return Ok(None);
         };
-        // Resolve chunk pointers transparently. Direct BlobEnvelope values
-        // pass through unchanged; pointer values fan out to chunk entries
-        // in the same store. Missing / malformed / hash-mismatched chunks
-        // are corrupt platform state — spec 9.3 (line 6272) calls this an
-        // internal config-store error with re-push remediation, NOT a
-        // transient unavailable. Mapping to `internal` surfaces as HTTP
-        // 500 and pushes operators toward `<app-cli> config push` instead
-        // of waiting for a 503 to clear.
+        // Resolve chunk pointers transparently. Direct BlobEnvelope values and
+        // any other raw value pass through; pointer values fan out to chunk
+        // entries in the same store.
+        //
+        // A chunk fetch can fail in two classes that must NOT collapse to one
+        // status:
+        //   - Corrupt state (a hash mismatch, a bad/oversized derived key) →
+        //     Internal (HTTP 500) with re-push remediation, per spec 9.3.
+        //     Re-pushing rewrites the generation and fixes it.
+        //   - Transient (invalid store handle, lookup exhaustion, an unclassified
+        //     error, a value that outgrew the read buffer, OR a referenced chunk
+        //     not yet visible at this POP) → Unavailable (HTTP 503, retryable).
+        //
+        // A MISSING referenced chunk is deliberately transient. Config Store is
+        // eventually consistent ACROSS keys, so right after a push the flipped
+        // root pointer can be visible at a POP before all of its
+        // content-addressed chunks have propagated there. That gap is not
+        // corruption — a retry moments later resolves it — so it must read as 503,
+        // not a re-push-me 500. (Genuine, lasting corruption then shows as a
+        // persistent 503 the operator repairs by re-pushing; that is strictly
+        // safer than a spurious 500 during the normal propagation window.)
+        // `transient` records whether any chunk fetch hit that class so the outer
+        // result can pick the right status.
+        let transient = Cell::new(false);
         let resolved = resolve_fastly_config_value(key, value, |chunk_key| {
-            self.get_sync(chunk_key)
+            let got = match &self.inner {
+                FastlyConfigStoreBackend::Fastly(inner) => inner.try_get(chunk_key).map_err(|err| {
+                    if is_transient_lookup(&err) {
+                        transient.set(true);
+                    }
+                    // The pointer-controlled chunk key is not echoed (the resolver
+                    // adds a safe position locator); the SDK `err` carries no value.
+                    format!("config store lookup failed: {err}")
+                })?,
+                #[cfg(test)]
+                FastlyConfigStoreBackend::InMemory(data) => data.get(chunk_key).cloned(),
+            };
+            if got.is_none() {
+                // Referenced chunk absent at this POP: treat as propagation lag
+                // (transient) rather than corruption.
+                transient.set(true);
+            }
+            Ok(got)
         })
         .map_err(|err| {
-            log::warn!(
-                "Fastly config-store chunk resolution failed for `{key}`: {err}. \
+            if transient.get() {
+                log::warn!(
+                    "Fastly config-store chunk lookup for `{key}` was transiently unavailable: {err}"
+                );
+                ConfigStoreError::unavailable("config store temporarily unavailable")
+            } else {
+                log::warn!(
+                    "Fastly config-store chunk resolution failed for `{key}`: {err}. \
                      Re-run `<app-cli> config push` to repair the store."
-            );
-            ConfigStoreError::internal(anyhow::anyhow!(
-                "config store entry is corrupt or incomplete; re-run config push to repair: {err}"
-            ))
+                );
+                ConfigStoreError::internal(anyhow::anyhow!(
+                    "config store entry is corrupt or incomplete; re-run config push to repair: {err}"
+                ))
+            }
         })?;
         Ok(Some(resolved))
     }
 }
 
+/// Is a CHUNK lookup failure environmental (retry) rather than corrupt config
+/// (`config push` to repair)? Only a bad KEY names corrupt state a re-push
+/// rewrites; everything else — an invalid store handle, lookup exhaustion, an
+/// unclassified/future failure, or a value that outgrew the read buffer — is
+/// transient, because re-pushing cannot fix a request-scoped condition.
+fn is_transient_lookup(err: &LookupError) -> bool {
+    // `ValueTooLong` is TRANSIENT, not corruption: the SDK already retried with
+    // the reported buffer size, so a `ValueTooLong` reaching us means the value
+    // GREW between host calls (a concurrent write) — a race a retry resolves, not
+    // a re-push-me corruption.
+    !matches!(err, LookupError::KeyInvalid | LookupError::KeyTooLong)
+}
+
 fn map_lookup_error(err: &LookupError) -> ConfigStoreError {
     // `LookupError` is from the `fastly` crate; using a wildcard arm guards
     // against new variants being added in upstream point releases without
@@ -134,6 +176,53 @@ mod tests {
         assert!(matches!(err, ConfigStoreError::InvalidKey { .. }));
     }
 
+    /// A CHUNK lookup that fails transiently (lookup exhaustion, an invalid
+    /// store handle, an unclassified error, or a value that outgrew the read
+    /// buffer) must keep Unavailable semantics — re-pushing cannot repair a
+    /// request-scoped condition. Only a bad KEY is corrupt state a re-push fixes.
+    #[test]
+    fn transient_chunk_lookups_are_not_treated_as_corruption() {
+        assert!(is_transient_lookup(&LookupError::TooManyLookups));
+        assert!(is_transient_lookup(&LookupError::ConfigStoreInvalid));
+        assert!(is_transient_lookup(&LookupError::Other));
+        // ValueTooLong is TRANSIENT: the SDK already retried at the reported size,
+        // so it reaching us means the value grew between host calls (a race).
+        assert!(is_transient_lookup(&LookupError::ValueTooLong));
+        assert!(!is_transient_lookup(&LookupError::KeyInvalid));
+        assert!(!is_transient_lookup(&LookupError::KeyTooLong));
+    }
+
+    /// A referenced chunk that is ABSENT maps to Unavailable (HTTP 503), not
+    /// Internal. Config Store is eventually consistent across keys, so a flipped
+    /// root pointer can reach a POP before all its chunks propagate there; that
+    /// window is retryable, not a re-push-me corruption.
+    #[test]
+    fn a_missing_chunk_maps_to_unavailable_not_internal() {
+        use crate::chunked_config::prepare_fastly_config_entries;
+        use futures::executor::block_on;
+
+        // A real chunked value, but seed ONLY the root pointer -- the chunks it
+        // references are "not yet propagated" to this POP.
+        let envelope = {
+            use edgezero_core::blob_envelope::BlobEnvelope;
+            use serde_json::json;
+            serde_json::to_string(&BlobEnvelope::new(
+                json!({ "pad": "x".repeat(9_000) }),
+                "2026-01-01T00:00:00Z".to_owned(),
+            ))
+            .expect("envelope")
+        };
+        let entries = prepare_fastly_config_entries("app_config", &envelope).expect("expand");
+        let (root_key, pointer_json) = entries.last().expect("pointer").clone();
+        let store = FastlyConfigStore::from_entries([(root_key.clone(), pointer_json)]);
+
+        let err = block_on(store.get(&root_key)).expect_err("a missing chunk must error");
+        assert!(
+            matches!(err, ConfigStoreError::Unavailable { .. }),
+            "a not-yet-propagated chunk must be retryable (Unavailable), not Internal: {err:?}"
+        );
+    }
+
     /// Spec 9.3 (line 6272): missing chunks, hash mismatches, pointer
     /// parse failures, and full-envelope mismatches are CORRUPT PLATFORM
     /// STATE — the runtime returns an internal config-store error with
@@ -142,14 +231,13 @@ mod tests {
     #[test]
     fn corrupt_chunk_pointer_maps_to_internal_not_unavailable() {
         use futures::executor::block_on;
-        // A root value that is neither a valid BlobEnvelope nor a valid
-        // FastlyChunkPointer triggers the resolver's "malformed pointer"
-        // path. We do NOT add the chunks the (would-be) pointer expects,
-        // so any chunk fetch would also fail — but the resolver bails
-        // earlier on the parse failure.
+        // A root value that ANNOUNCES our chunk-pointer kind but is malformed.
+        // It must be a pointer-kind value: an unrelated raw value is a
+        // legitimate Config Store entry and passes through untouched, so it
+        // would not exercise the corruption path at all.
         let store = FastlyConfigStore::from_entries([(
             "app_config".to_owned(),
-            "not-a-valid-envelope-or-pointer".to_owned(),
+            r#"{"edgezero_kind":"fastly_config_chunks"}"#.to_owned(),
         )]);
         let err = block_on(store.get("app_config"))
             .expect_err("corrupt root must map to a ConfigStoreError");

crates/edgezero-adapter/src/registry.rs

@@ -204,6 +204,15 @@ impl<'entry> TypedSecretEntry<'entry> {
 /// Outcome of a single-key read. See spec 9.0.
 #[non_exhaustive]
 pub enum ReadConfigEntry {
+    /// The entry EXISTS but its stored value cannot be resolved to a usable
+    /// envelope — a missing chunk, a hash mismatch, a malformed/foreign pointer.
+    /// Distinct from a read/IO failure (which still errors): the store WAS
+    /// reachable and the key WAS present, so a `config push` can safely overwrite
+    /// it. This is the runtime's "re-run config push to repair" contract made
+    /// real — the push treats it like an absent remote (proceeds to write) rather
+    /// than aborting, so a broken generation is recoverable in-band. The
+    /// `&'static str` carries a redacted, human-readable reason.
+    Corrupt(&'static str),
     /// The store exists but the key is absent (operator hasn't pushed yet,
     /// or pushed under a different key).
     MissingKey,
@@ -243,6 +252,51 @@ pub trait Adapter: Sync + Send {
     /// Returns an error string if the requested adapter action fails.
     fn execute(&self, action: AdapterAction, args: &[String]) -> Result<(), String>;
 
+    /// Reclaim chunk entries that no LIVE config pointer references.
+    ///
+    /// Deliberately NOT part of `config push`. On an eventually-consistent
+    /// store, a chunk may only be deleted once the pointer that referenced it
+    /// has stopped being served everywhere — and the platform may record no
+    /// pointer-supersession time (Fastly does not: `updated_at` is not bumped
+    /// by an upsert) and offer no compare-and-swap with which to record one
+    /// safely. Only the OPERATOR knows their deploy history, so `older_than`
+    /// carries that assertion.
+    ///
+    /// **`older_than` is a STORE-WIDE assertion, and it is stronger than "old
+    /// creations are no longer served".** `config gc` sweeps every root in the
+    /// selected store, so the caller asserts: **no root in this store changed
+    /// within the window, AND no writer is targeting the store** while `gc` runs.
+    /// A recently-changed sibling root — especially one that shrank to a direct
+    /// value, leaving no live chunk to age by — makes a wide window unsafe. Direct
+    /// trait callers MUST honour this stronger contract, not the weaker
+    /// paraphrase. A `dry_run` lists exactly what would be deleted, for review.
+    ///
+    /// # Errors
+    ///
+    /// Returns `Err` if the adapter has no `config gc` impl, if the platform
+    /// state cannot be read, or if it cannot be classified with confidence —
+    /// reclamation FAILS CLOSED: when in doubt, nothing is deleted.
+    #[inline]
+    #[expect(
+        clippy::too_many_arguments,
+        reason = "mirrors `push_config_entries`: manifest root, adapter manifest path, component selector, resolved store, push-time overlay, the operator's age assertion, and dry-run — each distinct; an aggregate struct is a worse ergonomic trade for implementers."
+    )]
+    fn gc_config_entries(
+        &self,
+        _manifest_root: &Path,
+        _adapter_manifest_path: Option<&str>,
+        _component_selector: Option<&str>,
+        _store: &ResolvedStoreId,
+        _push_ctx: &AdapterPushContext<'_>,
+        _older_than_secs: u64,
+        _dry_run: bool,
+    ) -> Result<Vec<String>, String> {
+        Err(format!(
+            "adapter `{}` does not implement `config gc`",
+            self.name()
+        ))
+    }
+
     /// Store kinds whose logical-id namespaces the adapter merges into
     /// a single backend at runtime — declaring the SAME logical id
     /// under two merged kinds causes silent write collisions because
@@ -263,6 +317,26 @@ pub trait Adapter: Sync + Send {
     /// Name used to reference the adapter (case-insensitive).
     fn name(&self) -> &'static str;
 
+    /// Reject a config `(key, body)` that this adapter cannot store, BEFORE any
+    /// provider I/O. Called by `config push` ahead of the remote read, so an
+    /// infeasible push fails offline instead of after a `list`/`describe`
+    /// round-trip.
+    ///
+    /// `body` is the serialised blob-envelope JSON that would be written, so
+    /// body-dependent feasibility (e.g. Fastly's derived chunk-key length and
+    /// pointer-size limits, which depend on whether the value chunks) can be
+    /// checked here rather than during the later write expansion.
+    ///
+    /// Default: accept. The Fastly adapter overrides this to reject a reserved
+    /// or over-limit key and to run the full chunk expansion offline.
+    ///
+    /// # Errors
+    /// Returns a human-readable error string if the push is infeasible.
+    #[inline]
+    fn preflight_config_write(&self, _key: &str, _body: &str) -> Result<(), String> {
+        Ok(())
+    }
+
     /// Provision the platform resources backing each store id the
     /// user declared. Returns a list of human-readable
     /// status lines the CLI logs verbatim — one line per resource