Conversation
Fastly SDK 0.12.1 exposes `fastly::http::serve::Serve`, which lets one Wasm
sandbox serve several requests. Adopt it behind a `reusable-sandbox` feature
that is off by default, so the shipped entry point is unchanged.
Split `main` into a loop owner and `handle_request`. The handler keeps sending
its own response and returns `()`, which the SDK's `HandlerResult` impl treats
as already sent, so progressive streaming, duplicate `Set-Cookie` headers,
response extensions, and post-send pull sync are untouched. No application
state is retained yet; that follows in a later change.
Three execution modes, only the first identical to today:
feature off -> original entry path
feature on, effective max_requests <= 1 -> single-request handler path
feature on, validated reuse limits -> Serve loop
Bounds come from the `edgezero_runtime_env` config store. They cannot be read
through `runtime_env_config`, whose `runtime_env_keys` allowlist drops every
key outside the adapter, logging, and per-store selectors; routing them there
would read as absent and silently disable reuse while passing every test. The
adapter opens the store itself and builds the service-scoped key from
`service_id()`, since edgezero's own key helper is private. Reads use
`try_get`, never `get`: `get` panics on a lookup error, and this runs before
the health probe. Any failure resolves to single-request operation.
Reuse needs all three bounds. A bare request limit is refused because the SDK
reads an omitted lifetime and wait timeout as `Duration::MAX`, and an
application-level `0` normalizes to `1` because the SDK reads
`with_max_requests(0)` as unlimited.
Guard the global logger. `fern`'s `apply()` panics when a logger is already
installed, which a reused sandbox would hit on its second request. Startup
diagnostics are deferred and flushed once the logger exists, since limit
resolution runs before there is anywhere to log.
Add measurement so the lifecycle can be evaluated rather than assumed. A
`sandbox_metrics_enabled` debug flag attaches instance id, request ordinal,
build count, and correlation id to workload responses before headers commit;
post-commitment stream failures carry the same context in logs. The flag is
skipped while false because `DebugConfig` denies unknown fields and a default
blob must stay readable by an older binary. The counters endpoint is feature
gated and short-circuits ahead of application construction so polling cannot
perturb the build count it reports. Response counters stay feature independent
so the feature-off baseline is measurable on the same channel.
Add `ts dev sandbox-probe`, which issues a keep-alive sequence and reports
reuse only from strictly increasing ordinals under one instance id. Repeated
or decreasing ordinals, incomplete attribution, a missing instance identity,
and transport errors all report unverified rather than a negative result.
`test-fastly` does not pass `--all-features` and CI invokes it directly, so
add a `test-fastly-reuse` alias and CI step; clippy compiling the feature is
not the same as running its tests.
`GoogleTagManagerIntegration` and `NextJsNextDataRewriter` accumulate inline script fragments across `lol_html` chunk boundaries, draining only when `is_last_in_text_node` arrives. Both held that buffer as a `Mutex<String>` field on the rewriter itself. The registry stores rewriters as `Arc<dyn IntegrationScriptRewriter>`, registered once at build time, so the buffer lives as long as the registry. When a document's stream ends before the final fragment — client disconnect, origin error, truncated body — its partial script stays in the buffer, and the next document through that registry prepends the residue to its own accumulation. That corrupts the response and can disclose the previous document's content. Today each Fastly sandbox serves one request, so the buffer is destroyed before anything else can observe it. Retaining the registry across requests is what makes it reachable, so this has to land before retention does. Add `ScriptTextAccumulator` and store it in `IntegrationDocumentState`, which is already constructed per document and already threaded through `IntegrationScriptContext`. Keyed per integration id, so each integration gets its own buffer and it is dropped with the document. The rewriter objects become stateless. `NextJsRscPlaceholderRewriter` deliberately does not accumulate and is unaffected. Both regression tests drive one rewriter through two documents, interrupting the first mid-accumulation, and assert the second carries no trace of it. Against the previous code they fail with the first document's content prepended to the second document's response.
Build the application lazily, once per sandbox, and keep it in `Sandbox`. This is what actually amortizes the initialization the earlier commits made safe to share: settings load and parse, secret resolution, auction plan compilation, orchestrator and integration registry construction, and the telemetry sink. The `OnceLock` regex caches in `settings.rs` now survive past the request that populated them. Construction sits behind the health, counters, and JA4 short-circuits, so none of those pays for it. A failed build is never retained. `build_app_with_state` returns an error router with no state; that serves the current request and is dropped, so a transient config-store failure cannot pin the sandbox into permanent error mode, and the next request retries. The attempt is still counted, so a retry loop shows up in the build counter rather than hiding. Nothing request-scoped is retained. The config store handle, client info, device signals, TLS metadata, `RuntimeServices`, correlation id, EC finalize state and request filter effects are all still built per request. Everything reachable from the retained `AppState` was audited as config-derived, and the script accumulation buffers that were not are fixed in the preceding commit. With no reuse configured this changes nothing observable: a single-request sandbox builds once, serves once, and exits. Retention does not remove the per-request Ed25519 signing key parse, which runs from `RequestSigner::from_services` against the per-request `RuntimeServices` inside the retained orchestrator. That needs its own measurement and rotation decision.
Issue #856 asked for CPU and memory observations alongside build counts and latency. Add them to the sandbox counters: cumulative guest vCPU milliseconds from `elapsed_vcpu_ms` and the heap snapshot from `heap_memory_snapshot_mib`, each reported as `unsupported` rather than a zero when the host lacks the call, so an unsupported counter is never mistaken for an idle one. Take the instance id from `compute_runtime::sandbox_id()` instead of reading `FASTLY_TRACE_ID` directly. The SDK documents that function as the per-sandbox identifier and resolves it to `FASTLY_TRACE_ID` on wasm32-wasip1, so this is the same value by the supported API, and it keeps working on the component path where the environment variable does not exist. Record the local A/B/C measurement in the results document: commands, commit ids, configuration, raw observations, and limitations. Headline: retention takes builds per sandbox from 6 to 1. Reuse without retention does not (arm B rebuilds on all 6). A reused request is 11.7x faster than the feature-off baseline at p50 on an edge-only route, and about 3.9x averaged over a full sandbox once the one build is amortized. Progressive delivery, duplicate Set-Cookie, and request isolation are each verified on observed reused instances. Failed builds are confirmed not retained; recovery after a failure in the same sandbox is covered only at unit level, because Viceroy's stores are fixed for a guest's lifetime. Long-lived memory behaviour and deployed eviction remain unverified, and the six-requests-per-sandbox figure is a repeated local observation under Viceroy 0.17.0 rather than a demonstrated universal ceiling.
The recovery test drove the `Sandbox` API by hand: it incremented the build counter and inserted an application itself, never invoking the production build/reuse/retry decision. It would have passed with that decision broken, so it was not evidence for the recovery claim made in the results. Extract the decision into `Sandbox::resolve_app`, which takes the builder as a closure. `edgezero_main` now calls it instead of open-coding the same logic, and the tests drive it with an injected builder. Two tests replace the hand-driven one. Reuse: build once, then three requests whose builder panics if called. Recovery: a failing build, then a succeeding one, then a third request whose builder panics if called — so recovery is shown to be both reachable and durable. Both were checked against deliberate mutations of `resolve_app` (never reusing the retained app, and swallowing the failed build) and fail as intended. Results document corrections: - Recovery is described as unit-level only, naming the new test and why an end-to-end same-sandbox recovery is not locally reproducible. - The vCPU and heap numbers are labelled pre-send cumulative samples. They are read before headers commit, so the first excludes that request's remaining work, consecutive differences straddle two requests, and the heap figure is linear memory including host buffering rounded to MiB, not Rust heap usage. - The aggregate comparison uses like-for-like sample means (3.872 / 0.934 ≈ 4.15x) instead of a modelled six-request average against a median. The modelled amortization is kept but labelled as a model. - Resource-measurement provenance is recorded: those headers landed in f51ba6f, not in the arm C commit, they were read with curl rather than the probe, and the latency experiment was not rerun on that revision. Also drop the stale "no application state" comment on `Sandbox`, which stopped being true when retention landed.
`ts dev sandbox-probe` builds on all non-wasm hosts and writes through `crate::output`, but that module was gated to macOS because the macOS-only `ts dev proxy` was its first consumer. On Linux the probe therefore referenced a module that did not exist, and CI failed with `cannot find output in crate` in three jobs. Widen the gate to `not(target_arch = "wasm32")`, matching `commands`, `run`, and the crate's other host-only modules. Nothing in `output` is platform specific: it is a `println!` / `eprintln!` wrapper and the only place in the crate permitted to write to the console. This was not caught locally because the host-target CLI clippy was run with the macOS triple; CI hardcodes `x86_64-unknown-linux-gnu`.
`output::warn` had no caller outside the macOS-gated proxy, so widening `mod output` to every host target left it dead on Linux and CI failed with `function warn is never used`. The probe should have been warning anyway. A transport error invalidates the run, and the report goes to stdout, so a caller piping stdout to a file would otherwise lose that signal entirely. Emit it on stderr as well. An earlier draft of the probe did call `warn`; the rewrite onto hyper replaced that path with the `Ending` enum and dropped the call without noticing.
Move all six EdgeZero dependencies from `tag = "v0.0.8"` to `rev = "277544c431c1ab9bafa14a45d5f35975b5587e97"` on `feat/reusable-app-lifecycle`. The lockfile changes 16 lines, all of them the `source` field of the eight edgezero packages; no other dependency moves, one distinct edgezero source resolves, and no `v0.0.8` reference survives. The repin is deliberately not for the `Serve` re-export. That type comes from the already-pinned `fastly 0.12.1` SDK, and EdgeZero's own custom-lifecycle contract says not to repin merely to swap the import; the entry point still calls `fastly::http::serve::Serve` directly. The pin is for three fixes at that revision: push/diff validation scoped to the selected adapter, secret references redacted from Spin diagnostics, and duplicate response headers preserved on Cloudflare. The first two are the CLI defects found while measuring this branch, where a Spin naming rule blocked a Fastly push and the error printed the rejected reference. No local code was removed. The contract allows removal only where a public EdgeZero API now provides equivalent behaviour; at this revision `service_scoped_runtime_env_key` is still private and `runtime_env_keys` is still a closed allowlist, so the local key construction remains necessary. Every `edgezero-core` change across the range is test-only and the Fastly adapter change is additive, so nothing we depend on moved. Re-verified fresh against this revision rather than carrying prior results: default stays single-request with limits configured, health and debug probes still bypass construction, a reused sandbox holds one successful build, failed initialization is retried every request and never retained, request state stays isolated, delayed chunks reach the client about two seconds before the origin finishes, duplicate Set-Cookie and finalization survive, and three consecutive post-commitment stream failures on one sandbox each produced exactly one response before a normal request succeeded on that same sandbox. EdgeZero's own Fastly compatibility suite passes at this checkout, including the custom-lifecycle arms. Documentation records the dependency diff, the CLI checks performed with synthetic values, the fresh observations, and the remaining gaps: in-guest initialization recovery is unit-level only for this application, Linux verification stays CI-only, comparative latency was not rerun, and the pin is an unmerged branch revision that should move to a release tag once available.
Repin EdgeZero to `76c59b440fb35d1317dcb3fa8c1172161e3f5309` on `feat/reusable-app-lifecycle`, which adds `edgezero_adapter_fastly::lifecycle`, and use it instead of this adapter's own equivalents. The lockfile moves only the eight edgezero packages' `source` fields; one distinct source resolves and no other dependency changes. Deleted here, now owned by the framework: local `struct Sandbox` -> `lifecycle::Sandbox<RetainedApp>` `resolve_app`/`retain_app`/`retained_app` -> `Sandbox::initialize` `ensure_logger` + `logger_installed` -> `Sandbox::setup_once` `begin_request`/`record_build`/... -> `requests()`/`initialization_attempts()` `Serve::new()…run_with_context(…)` -> `serve_custom` / `run_custom` `initialize` retains only success. A failed build hands its error router back as the error payload, which serves the current request and is dropped, so the next callback retries. Request ordinals now come from the framework, which counts a callback before invoking it, so probe short-circuits advance the ordinal without attempting construction — and no local counter shadows it. `logging::init_logger` returns `Result` rather than panicking on the install path, so `setup_once` marks setup complete only after a successful install. That allows a retry; it does not by itself make retrying safe, since `setup_once` rolls nothing back. It is safe here because neither failure mode leaves the process partially configured: a failed builder installs nothing, and a failed `apply()` means a global logger already exists so the retry fails identically. The function's docs record that reasoning. `serve_custom` owns the `Sandbox` and drops it when serving ends, so the retirement line reports snapshots the callback captured rather than reading the sandbox afterwards. `serve_app` is deliberately not adopted: its response conversion buffers streams, which would end progressive delivery and leave no place for response-extension finalization. Application-owned and unchanged: feature gate, kill switch, limit parsing and fallback, settings retention and refresh, health/JA4/metrics routing, fresh per-request metadata, handles, services, extensions, bodies and correlation ids, raw request conversion, router dispatch, response-extension finalization, progressive streaming, duplicate Set-Cookie, post-send work, and per-document rewrite-buffer isolation. One new local type, `StartupDiagnostics`, holds messages produced by limit resolution before any logger exists; it cannot live in the framework `Sandbox`, which exposes no slot for application state other than the retained payload. Verified fresh at this revision on macOS: feature-off stays single-request with limits configured; lazy start with six requests and one build; probes counted but not constructing; failed initialization retried five times and never retained; request isolation; duplicate Set-Cookie and finalization; progressive delivery about two seconds ahead of origin completion; and two post-commitment failures each producing exactly one response before a normal request succeeded on the same sandbox. EdgeZero's own Fastly compatibility suite exits 0 at this checkout. Limitations recorded in the results document: same-sandbox initialization recovery stays unit-level because Viceroy's stores are fixed for a guest's lifetime; Linux remains CI-only and is not claimed to pass from these macOS runs; long-lived memory and deployed eviction stay unverified; no comparative latency was rerun at this revision. The pin is an unmerged branch revision and should move to a release tag once one contains it.
`serve_loop` snapshotted `initialization_attempts()` before invoking the callback. Initialization happens during the callback, so a build performed by the final callback was missing from the retirement line. The count itself was never lost — it lives in EdgeZero's `Sandbox` until serving ends — but the snapshot was stale by the time that callback finished. `requests()` on entry was already correct: `lifecycle::Sandbox::handle` increments its callback count before invoking the handler, so the value read on entry is the current callback's 1-based ordinal. Extract `RetirementCounters::observe`, which reads `requests()` on entry and `initialization_attempts()` on exit, and wrap every callback in it. Extracting rather than reordering one line means the regression tests drive the same method production uses, instead of restating the ordering in a test where it could drift. Both values are still read from EdgeZero's counters; nothing here increments anything. Three regressions cover it: a successful build on the final callback, a failed build on the final callback, and that the request snapshot mirrors the framework counter rather than deriving one. The failed case matters because the application state is not retained, so the attempt count is the only record that the build happened. Reintroducing the entry-read ordering fails the first two. `RetirementCounters` is gated to the reuse feature, matching `scoped_key` and `sandbox_metrics_response`, so the default build stays warning-free. Also record a follow-up on `sandbox::scoped_key`, which reproduces EdgeZero's private runtime-store key format. A public key-construction or lookup helper would remove the duplication; the working implementation stays until one exists, and the `TS__SANDBOX__*` suffixes and limit-validation policy remain application-owned either way.
prk-Jr
changed the title
`SANDBOX_METRICS_PATH` and `sandbox_metrics_response` were gated `#[cfg(any(feature = "reusable-sandbox", test))]`, but their only caller is the short-circuit gated on the feature alone. A feature-off test build therefore compiled both without a caller, and CI failed on `dead_code`. Gate them on `feature = "reusable-sandbox"` to match their call site. The other `any(feature, test)` gates in this module stay: `resolve_mode`, `collect_raw_limits`, `scoped_key`, the key constants and `RetirementCounters` are all exercised by tests that run in both feature configurations. This did not reproduce locally because CI's `cargo test` job uses `actions-rust-lang/setup-rust-toolchain`, which defaults `RUSTFLAGS` to `-D warnings`. The `test-*` aliases carry no such flag, so warnings that fail CI are merely printed locally. Verified this fix under `RUSTFLAGS="-D warnings"` across every test alias, the CLI suite, the parity suite, and all clippy invocations.
aram356
marked this pull request as draft
September 17, 2026 15:29
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
reusable-sandboxCargo feature (whichfastly.tomldoes not pass) and explicit bounds in the runtime config store. Absent or partial configuration stays single-request, so the shipped build is unchanged.What changed, in order
The branch is meant to be read commit by commit.
c56ff745emainsplit, logger guard, measurement counters,ts dev sandbox-probe.00c4e4278ca7e7fa7af51ba6f510b447f804dd74fa202,66529f8f1outputmodule on Linux).b6df0b3e9277544c4(CLI + Cloudflare fixes).281c89a98edgezero_adapter_fastly::lifecycle, deleting our equivalents. Pin76c59b44.52942ba8bA cross-request disclosure fix, worth reading on its own
GoogleTagManagerIntegrationandNextJsNextDataRewriteraccumulated inline script fragments in aMutex<String>on the rewriter, which the registry holds for its whole lifetime. If a document's stream ended before its final fragment — client disconnect, origin error, truncated body — the partial script stayed in that buffer and the next document prepended it, corrupting the response and potentially disclosing the previous document's content.This is latent today only because each sandbox serves one request and then dies. Retention is what makes it reachable, so it is fixed in its own commit (
00c4e4278) ahead of retention. Both regression tests fail against the previous code with the first document's content prepended to the second's response.What EdgeZero owns vs what we own
After
281c89a98the framework owns lazy successful-only retention, the callback count, the initialization-attempt count, the one-time setup guard, and the serving wrappers. Deleted here:struct Sandboxlifecycle::Sandbox<RetainedApp>resolve_app/retain_app/retained_appSandbox::initializeensure_logger+logger_installedSandbox::setup_oncebegin_request/record_build/ …requests()/initialization_attempts()Serve::new()…run_with_context(…)serve_custom/run_customStill ours, deliberately: feature gate, kill switch, limit parsing and fallback; settings retention and refresh policy; health/JA4/metrics routing; fresh per-request metadata, handles, services, extensions, bodies and correlation ids; raw request conversion and router dispatch; response-extension finalization; progressive streaming; duplicate
Set-Cookie; post-send work; per-document rewrite-buffer isolation.serve_appis not adopted: its response conversion buffers streams, which would end progressive delivery and leave nowhere for response-extension finalization.Dependency
EdgeZero pinned by SHA to
76c59b440fb35d1317dcb3fa8c1172161e3f5309onfeat/reusable-app-lifecycle.Cargo.lockmoves only the edgezero packages'sourcefields; no other dependency changes. The pin is an unmerged branch revision and should move to a release tag once one contains it.Closes
Closes #856
Test plan
cargo test-fastly && cargo test-axum && cargo test-cloudflare && cargo test-spincargo test-fastly-reuse— new alias;test-fastlydoes not pass--all-features, so feature-on tests would otherwise never runcargo fmt --all -- --checkLocal evidence
The middle row is the control: the serving loop alone buys almost nothing. The gain is retention.
Verified on observed reused sandboxes: progressive delivery (first byte ~2 s ahead of origin completion), duplicate
Set-Cookie, request isolation with distinct per-request metadata, probes not constructing the app, failed initialization retried and never retained, and post-commitment stream failures producing exactly one response before a normal request succeeds on the same sandbox.Full method, raw observations and provenance:
docs/superpowers/specs/2026-09-17-fastly-reusable-sandbox-results.md.Limitations — deployed behaviour is not established
Rollback
No rung is immediate — limits are read at sandbox startup, so a running sandbox retires on the limits it started with.
Checklist
unwrap()in production codelogmacros (notprintln!)