Repository navigation
Extract native prepared startup on the demo baseline - #1531
ChristianPavilonis wants to merge 29 commits into
Conversation
prk-Jr
left a comment
There was a problem hiding this comment.
Summary
REQUEST_CHANGES: one blocking finding at b121676, reviewed against the frozen demo baseline 2dd968a.
Blocking
- 🔧 [P1] Start native bidder requests before returning pending handles — see inline at
crates/trusted-server-adapter-axum/src/outbound.rs:250–253.
Verification and limits
An isolated probe against the exact pinned EdgeZero native client confirmed that requests make no connection before batch collection, work when collection begins, and expire without contacting the server when collection is delayed 80 ms against a 40 ms budget. The complete publisher consequence is source-traced; the probe executed the underlying transport behavior.
Local provider-helper tests passed under pinned Node; shell syntax and diff checks passed. Full Rust suites were not rerun. Real provider acceptance and Spin host/interface qualification remain unverified here; large unchanged source/test sections were sampled around affected consumers. CodeQL is main-base-only and did not run for this historical baseline.
CI Status
All 18 reported checks pass for the reviewed revision. No required checks are reported for this baseline branch.
- integration tests: PASS
- integration tests (Fastly EC lifecycle): PASS
- browser integration tests: PASS
- format-typescript: PASS
- docs-proposal scripts: PASS
- CLAUDE.md symlink guard: PASS
- format-docs: PASS
- cargo fmt: PASS
- prepare integration artifacts: PASS
- cargo check/build/test (spin native + wasm32-wasip2): PASS
- cargo test: PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo test (cross-adapter parity): PASS
- cargo test (ts CLI, native) (ubuntu-latest): PASS
- cargo test (ts CLI, native) (macos-latest): PASS
- vitest: PASS
- cargo test (axum native): PASS
- PBS Terraform and Compose checks: PASS
| let batch = self.client.start_batch_until(vec![outbound], deadline); | ||
| let handle = NativePendingHandle { | ||
| backend_name: backend_name.clone(), | ||
| completion: finish_batch(batch).boxed_local(), |
There was a problem hiding this comment.
🔧 [P1] Start native bidder requests before returning pending handles
send_async stores an unpolled batch future. The pinned EdgeZero native client does no network work until that future is polled. Publisher navigation dispatches bids, awaits the publisher origin, then collects bids. Consequently, bidder traffic cannot overlap the origin fetch; an origin response taking longer than the auction budget causes bids to expire before bidders are contacted. The previous implementation started requests immediately.
An isolated probe against the exact dependency confirmed:
accepted_before_collection=false
accepted_during_collection=true
delayed_collection_termination=Cutoff, successful=false, accepted=false
The delayed case waited 80 ms against a 40 ms budget. The complete publisher consequence is source-traced; the probe executed the underlying transport behavior.
Proposed correction: drive pending requests concurrently with origin processing while preserving cancellation and deadline ownership. Add a regression requiring bidder activity before origin completion. This needs lifecycle coordination and a behavioral test, so the remedy is prose-only.
Retain the existing native feature head and current-main cutover history. Build the application router only after bounded prepared config and secret snapshots pass runtime validation, preserving operator publication rules and application forwarding authorization. Use EdgeZero's production lifecycle and direct outbound path. Keep native EC and explicit storage for their later feature units. Preserve checked and unavailable ingress origin handling without restoring the old native HTTP bridge.
aram356
left a comment
There was a problem hiding this comment.
Summary
Reviewed at 65ba4f3df against the frozen baseline 2dd968aee, with EdgeZero source read at 83d7a629. The native prepared-store startup itself holds up: every invalid-input path we traced or probed fails before the listener binds, there is no legacy env fallback, snapshots are bounded and immutable, and ingress trust (host, scheme, client IP) cannot be spoofed by a direct client. The blocking issues are behaviour changes this unit introduces on the request path (413 to 400 on every non-Axum adapter, a new 8 MiB upload cap reported as 502, native responses served uncompressed), production wiring that tests don't reach, and harness checks that can pass without proving anything.
10 of the inline comments carry a one-click GitHub
suggestion. Use Commit suggestion (or Add suggestion to batch for several at once) to apply them. Each was applied alone to this head and checked; see "Verification" below. The remaining comments describe the fix in prose because it spans files or needs new tests.
Blocking
🔧 wrench
- Oversized API bodies now return 400 instead of 413, and skip finalize headers (all adapters): see inline at
crates/trusted-server-adapter-fastly/src/app.rs:578 - New 8 MiB default upload cap turns large publisher POSTs into 502: see inline at
crates/trusted-server-core/src/platform/http.rs:97 - Native responses served decoded under the origin's gzip ETag: see inline at
crates/trusted-server-adapter-axum/src/outbound.rs:108 - Fastly delivery tests bypass the production gate, finalizer and observer: see inline at
crates/trusted-server-adapter-fastly/src/delivery_tests.rs:47 - Spin snapshot install and streamed-body buffering are untested (mutation survivors): see inline at
crates/trusted-server-adapter-spin/src/startup.rs:333 - Spin missing-secret diagnostics no longer name the secret: see inline at
crates/trusted-server-adapter-spin/src/startup.rs:170 - Gateway-isolation test fails on macOS (suggestion): see inline at
crates/trusted-server-adapter-axum/tests/native_acceptance.rs:579 - Self-connection fix missing from the startup no-listener probe (suggestion): see inline at
crates/trusted-server-adapter-axum/tests/startup.rs:101 - Provider acceptance script exits 0 without running via a symlinked path (suggestion): see inline at
scripts/test-native-providers.mjs:628 - Missing-config test cannot detect a legacy fallback: see inline at
crates/trusted-server-adapter-axum/tests/startup.rs:220 - Body-level: prk-Jr's P1, stale operator docs, CI rerun (below)
❓ question
- Is a 30 s response-write cutoff on every routed response intended?: see inline at
crates/trusted-server-adapter-fastly/src/app.rs:1324 - Should a signing inconsistency take down every Spin route, including
/health?: see inline atcrates/trusted-server-adapter-spin/src/startup.rs:227
Non-blocking
🤔 thinking / ♻️ refactor / 🏕 camp site / ⛏ nitpick
- 🤔 Enforceable-deadline claim contradicts core and discards late mediator results (suggestion): see inline at
crates/trusted-server-adapter-axum/src/outbound.rs:295 - 🤔
x-edgezero-proxyheader reaches clients (suggestion): see inline atcrates/trusted-server-adapter-axum/src/outbound.rs:114 - 🤔 Reused Fastly sandbox retires on every post-commit delivery failure: see inline at
crates/trusted-server-adapter-fastly/src/main.rs:552 - 🤔 Spin lists and reads the whole default store on every invocation: see inline at
crates/trusted-server-adapter-spin/src/startup.rs:204 - 🤔 Budget tests construct an unreachable
timeout = Nonestate: see inline atcrates/trusted-server-adapter-axum/src/outbound.rs:344 - 🤔 Upload-collection deadline in
send_asynchas no test: see inline atcrates/trusted-server-adapter-axum/src/outbound.rs:195 - 🤔 Truncated-POST check cannot see an aborted upload delivered as complete (suggestion): see inline at
crates/trusted-server-adapter-axum/tests/native_acceptance.rs:705 - 🤔 Missed adapter buffer surfaces as a client 400 with an internal message: see inline at
crates/trusted-server-core/src/auction/endpoints.rs:151 - 🤔 Mutable git
tag =on a frozen baseline: see inline atCargo.toml:59 - 🤔
native-rc-demo.mdis an orphan page written as a review note: see inline atdocs/guide/native-rc-demo.md:1 - 🤔 Spec contradicts this unit: see inline at
docs/superpowers/specs/2026-10-20-edgezero-431-compatibility-design.md:5 - 🤔 Cloudflare smoke no longer runs on macOS or with asdf/mise: see inline at
scripts/smoke-cloudflare.sh:15 - ♻️
enforce_basic_auth_partshas no tests: see inline atcrates/trusted-server-core/src/auth.rs:95 - 🏕 Restore the dropped marker rationale (suggestion): see inline at
crates/trusted-server-core/src/auth.rs:83 - ⛏ Enforce "no environment proxy" (suggestion): see inline at
scripts/test-native-providers.mjs:132 - ⛏ Stale Spin target comment (suggestion): see inline at
.cargo/config.toml:56 - ⛏ Doc summary order on
MAX_AUCTION_BODY_SIZE(suggestion): see inline atcrates/trusted-server-core/src/auction/endpoints.rs:46 - ⛏ Fastly finalize and log cleanups: see inline at
crates/trusted-server-adapter-fastly/src/main.rs:503
Cross-cutting / body-level findings
-
🔧 prk-Jr's P1 (lazy
send_async) is still open at this head. The last two commits only swaprev =for a tag resolving to the same83d7a629, sooutbound.rs:243-255is unchanged, and EdgeZero's Axumstart_batch_untilstill builds unpolledasync move { client.execute(..) }futures. No test catches it (separate_rc_batches_progress_concurrently_during_selectpasses only becauseselectpolls both handles). This regression test fails at head and passes with an eager control, so it can land with the fix intests/native_http.rs:#[tokio::test] async fn send_async_dispatches_before_select_or_wait() { let listener = tokio::net::TcpListener::bind("127.0.0.1:0") .await .expect("should bind eager-dispatch origin"); let address = listener.local_addr().expect("should read origin address"); let client = bridge(); let pending = client .send_async(request(&format!("http://{address}/bid"), "eager")) .await .expect("should start pending request"); let accepted = tokio::time::timeout(Duration::from_millis(500), listener.accept()).await; assert!( accepted.is_ok(), "should start bidder transport at send_async, before select or wait polls it" ); drop(pending); }
-
🔧 Operator docs still describe the startup this PR removes (all outside the diff):
docs/guide/axum-dev.md:9-41andcrates/trusted-server-adapter-axum/README.md:7-12document theTRUSTED_SERVER_CONFIG_*/TRUSTED_SERVER_SECRET_*env bridge and "every route returns 500"; the binary now exits before listening, andsmoke-axum.shsets those variables as poison.docs/guide/spin.md:9-16,40-43documents a 503 router with/healthkept up and signing unavailable; the Spin README itself now calls those expectations invalid.docs/guide/api-reference.md:21,24matrix rows for axum and spin.edgezero.tomldropped[stores.kv], butdocs/guide/fastly.md:358-362,docs/guide/edgezero.md:30andtrusted-server.example.toml:78-80still tell operators to provisiontrusted_server_kv.AGENTS.md:22still calls the Spin artifact awasm32-wasip1component;docs/guide/architecture.md:66,81,166anddocs/guide/testing.md:23,29also still say wasip1 for Spin.
-
🔧 CI did not complete on this head.
prepare integration artifactsfailed whenactions/upload-artifactreceived HTML from the artifact service (an outage, not a code failure), sointegration tests,integration tests (Fastly EC lifecycle)andbrowser integration testswere skipped. This head changes the browser job (--with-depsdropped,ubuntu-24.04), and those jobs have not run on it. Please re-run before merge. -
♻️ Run the new Node tests in CI. No workflow runs
scripts/tests/test-native-providers.test.mjsorscripts/tests/test-smoke-cloudflare-home.test.mjs(the latter also skips off Linux);node --test scripts/tests/in an existing ubuntu job would cover them. The smoke scripts, the provider acceptance run and the Spin 4.1 host run are local-only; please say in the description which of those were run on this head. -
♻️ Unused Axum dependencies:
reqwestin[dependencies](src/no longer uses it; RC brings its own) and thetemp-envandtowerdev-dependencies (crates/trusted-server-adapter-axum/Cargo.toml:33,45,47). -
🤔 Native harness assertions that could be stronger:
- Redaction checks skip the short EC passphrase (
"short",native_acceptance.rs:236-238,419-425,startup.rs:84-86,131-138) and never check success-path logs; use a sentinel value and assert absence everywhere. - Nothing proves bounded SIGTERM:
smoke-axum.sh:22-43andtest-native-providers.mjs:203-226silently escalate to SIGKILL (the binary exits 0 about 6 ms after SIGTERM today, so a strict assertion would pass). - Provider failure cases (
test-native-providers.mjs:397-413) run withstdio: 'ignore'andRUST_LOG: off, so they don't assert their cause ("startup config: … missing resource or version") or that no secret value is emitted; buffer the output in memory and check both. - Provider containers have unrestricted egress;
--dns 127.0.0.1on bothdocker runcalls would enforce the account-free claim.dockerEnvdropsDOCKER_HOST/DOCKER_CONTEXT, which breaks rootless and colima setups. - The secret half of the snapshot test (
native_acceptance.rs:519-548) cannot change a running child's env, so it only proves restart adoption;startup.rs:246says "until restart" but never restarts.
- Redaction checks skip the short EC passphrase (
-
⛏ Smaller items:
- Cloudflare
logging.rs:32-35is fixed at Info with no override and doesn't setHooks::owns_logging(), a new log-volume and PII surface in Workers Logs. - On native,
/.well-known/trusted-server.jsonalways returns 500 and/verify-signaturealways returnsverified:false, because the JWKS store is rejected by the snapshot (app.rs:452-456); returning 501, as the admin key routes do, would make the unsupported consumer explicit. - Spin tests: the 128-key boundary is untested (
>to>=survives),missing_required_secret_…asserts onlyis_err(), the"static "guard scans onlystartup.rs, andseparate_invocations_…never callsinitialize()/handle.startup.rs:396-401initializelacks# Errors, andapp.rs:87AppState::from_requesthas no doc comment. - Axum visibility and docs:
NativeStoreSnapshotispubbut crate-internal (startup.rs:31),AxumPlatformHttpClientis exported at two paths,RequestInfo::from_request's doc doesn't say a pre-inserted extension now overrides every header and TLS source, and the header limits atoutbound.rs:71-72are magic numbers. - Many new assertions have no message (for example
native_http.rs:104,129-131,281-285,native_acceptance.rs:382-390,472,506,startup.rs:250,258-261), andRouterHarnessis duplicated inroutes.rs:12-43andparity.rs:18-49. environments/axum.rs:113-121attaches only the exit status on pre-listen failure; now that bad config is fatal before listen, attaching the last stderr lines would make CI failures diagnosable.smoke-axum.shstill uses the Python origin and lacks the upload and duplicate Set-Cookie check the Fastly and Cloudflare smokes gained;scripts/README.mdis stale for both..tool-versionsuses the keyspin-frameworkwhere main usesspin.
- Cloudflare
-
📌 Follow-ups:
- RustSec:
rustls 0.23.45(RUSTSEC-2026-0347, low; fixed in 0.23.46) andctutils 0.4.3(RUSTSEC-2026-0385; only viaaws-sigv4, absent from default and wasm graphs) are new at this head. - EdgeZero asks: pre-dispatch RC errors (URI parse, admission
Aborted) now send no response and retire a reused Fastly sandbox; detached 414/431 head-limit responses skip TS finalization; a typed delivery error would fix the sandbox-retirement finding; an encoded-passthrough response mode would fix the decoding finding. - The real Next.js action-after-truncated-POST reproduction exists only as an
#[ignore]test CI never runs, and there are no native "disallowed origin" request tests (spec line 142). Please state whether the ignored test was run.
- RustSec:
-
🌱 Spin ignores the new
timeout,deadlineandmax_response_body_bytesfields (platform.rs:430-500; same as base), so a hung bidder still blocks the invocation even though providers now setwith_timeout. The test-onlyTRUSTED_SERVER__GIT_VERSIONfilter insettings.rs:3054-3062differs from main's #1326 fix (BUILD_ENVIRONMENT_VARIABLES) and will conflict when this baseline moves up.
Verification
- Each suggestion was applied alone to
65ba4f3dfand checked, each followed by a byte-exact drift check:- Rust:
cargo fmt --check, thenclippy-axum(orcheck-fastlyfor the core comment and doc changes) and the affected Axum test binaries. - JS:
node --check, the provider unit test, prettier, and a direct run of the script.
- Rust:
- All ten were then applied together and run through the full gate on aarch64-apple-darwin: fmt; the six adapter clippy lanes;
test-fastly(3,251),test-fastly-reuse(222),test-axumdefault and--all-features(77),test-cloudflare(55),test-spin(99) and parity (17). Everything passed with no drift. - The two test snippets in prose were run too: the
send_asyncregression test fails at head, and the upload-deadline test passes at head and fails against the mutant. - Spin findings come from
cargo test-spin, mutation runs and a Spin 4.1.0 host run of the wasm32-wasip2 artifact. Fastly findings come from Viceroy probes. The decoding and proxy-header findings come from a gzip origin through the real Axum bridge. - Not verified here: real AWS providers, the hosted browser and integration jobs (skipped by the outage above), and the ignored Next.js test.
CI Status
- cargo fmt: PASS
- cargo test: PASS
- cargo test (axum native): PASS
- cargo test (cross-adapter parity): PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo check/build/test (spin native + wasm32-wasip2): PASS
- cargo test (ts CLI, native) (ubuntu-latest): PASS
- cargo test (ts CLI, native) (macos-latest): PASS
- vitest: PASS
- format-typescript: PASS
- format-docs: PASS
- docs-proposal scripts: PASS
- CLAUDE.md symlink guard: PASS
- PBS Terraform and Compose checks: PASS
- prepare integration artifacts: FAIL (artifact service outage during upload)
- integration tests: SKIPPED
- integration tests (Fastly EC lifecycle): SKIPPED
- browser integration tests: SKIPPED
No required checks are reported for the baseline/native-aws-20261110 branch.
| _ => None, | ||
| }; | ||
| if let Some(limit) = limit { | ||
| ctx.body_bytes(limit).await?; |
There was a problem hiding this comment.
🔧 wrench: Oversized API bodies now return 400 instead of 413, and the error skips TS finalize headers
ctx.body_bytes(limit).await? returns EdgeZero's bad_request("request body too large") whenever the body carries no managed JSON policy (RequestContext::buffer_policy, edgezero-core context.rs:266). Only the Axum adapter attaches that policy, so on Fastly, Cloudflare and Spin every buffered route (auction, verify, rotate/deactivate, sign, rebuild, purge, batch sync) now answers 400 where base returned core's 413 (RequestTooLarge / PAYLOAD_TOO_LARGE). Core's own Content-Length pre-checks are now unreachable on those adapters.
Same pattern: trusted-server-adapter-cloudflare/src/app.rs:294, trusted-server-adapter-spin/src/app.rs:523,561,645,663, and trusted-server-adapter-axum/src/app.rs:164 for non-JSON bodies.
Two side effects:
- The
?exits throughFinalizeResponseMiddleware'snext.run(ctx).await?(Cloudflare, Spin, Axum), so these responses carry nox-ts-version, geo header or operatorresponse_headers. On Axum, a cross-origin 300 KiB JSON/auctiongets RC's 413 without the operator's CORS headers, which the browser then reports as a CORS failure. - On Fastly the body is now drained before batch sync's Bearer check and rate limit, and before cache purge's 415 content-type gate. An unauthenticated oversized batch sync gets 400 instead of 401, and an oversized non-JSON purge gets 400 instead of 415.
Router-level probes on all three adapters: a 300 KiB /auction (buffered and streamed) returned 400 {"error":{"kind":"bad_request","message":"request body too large"}} with no x-ts-version or X-Geo-Info-Available; a small streamed /auction returned 200 with both.
Proposed fix (apply manually; touches four adapters): handle overflow inside the route and render it through the TS error path so finalize headers apply. Key it on overflow rather than on every 400, because EdgeZero also replays a stored source BadRequest:
if let Some(limit) = limit
&& let Err(error) = ctx.body_bytes(limit).await
{
// Overflow is the only 400 a fresh drain produces today; see note above.
return if error.status() == StatusCode::BAD_REQUEST {
Ok(http_error(&Report::new(TrustedServerError::RequestTooLarge {
message: format!("request body exceeds {limit} bytes"),
})))
} else {
Err(error)
};
}Optionally reject a declared Content-Length above limit before draining, and keep Fastly's batch-sync auth and purge content-type checks ahead of the drain. Please add a 413 test per adapter; today only Axum's JSON case is covered (native_acceptance.rs:506). Longer term this is an EdgeZero ask: attach the inbound body policy on Fastly, Cloudflare and Spin, or expose an overflow kind on EdgeError.
| image_optimizer: None, | ||
| cache_intent: PlatformCacheIntent::Default, | ||
| stream_response: false, | ||
| max_request_body_bytes: 8 * 1024 * 1024, |
There was a problem hiding this comment.
🔧 wrench: The new 8 MiB default upload cap turns large publisher and integration POSTs into 502
PlatformHttpRequest::new now defaults max_request_body_bytes to 8 MiB, and every adapter enforces it: Fastly platform.rs:511 and :537, Cloudflare platform.rs:313 and Spin platform.rs:436 call buffer_body(), and the Axum bridge hands it to RC (outbound.rs:67). No caller overrides it, including the publisher fallback.
Base forwarded uploads of any size the runtime accepted (EdgeZero v0.0.9 buffered the inbound body without a TS cap, and the old Axum client buffered without one too). At head the overflow becomes PlatformError::HttpClient, then TrustedServerError::Proxy, which renders as 502 "Failed to proxy request to origin", so the client's own oversized upload is blamed on the origin. On Fastly the lazy inbound body also inherits RC's 30 s inbound read budget, so a slow mobile upload fails the same way.
Scenario: a 10 MiB CMS media upload or form POST through publisher passthrough returns 200 at base and 502 at head. A Fastly probe of send with an 8 MiB + 1 B body failed with http client error ╰─▶ request body too large before SDK dispatch.
Proposed fix (apply manually; core plus publisher and error mapping): choose the passthrough cap explicitly (a publisher setting, or no TS cap where the runtime already bounds it) and map cap and deadline failures to 413/408 instead of HttpClient. If 8 MiB is intended, document it for operators. Related: Fastly, Cloudflare and Spin ignore the new max_response_body_bytes, timeout and deadline fields (they keep fixed caps and no timeout), so with_response_limit/with_timeout in auction/provider.rs:386-391 only take effect on Axum.
| Report::new(PlatformError::HttpClient).attach(message.into()) | ||
| } | ||
|
|
||
| fn platform_response( |
There was a problem hiding this comment.
🔧 wrench: Native responses are served decoded (identity) under the origin's gzip ETag
RC always decodes gzip, br and deflate, and removes Content-Encoding and Content-Length (edgezero-core apply_response_body_policy, outbound.rs:1279-1295); there is no encoded-passthrough mode. TS still forwards the reader's Accept-Encoding to the origin (publisher.rs:291-312), and the publisher pipeline sets output compression equal to input compression (publisher.rs:814-820), which is now identity. Base used reqwest without decompression, so gzip reached core and was re-encoded for the client.
Probe through the real bridge with a gzip origin and Accept-Encoding: gzip, br:
/static/app.js: 200, nocontent-encoding, plain JS body,etag: "gzip-v1"/article: 200, identity HTML, the same gzip ETag, noVary: Accept-Encoding
Every native page and pass-through asset therefore ships uncompressed, and a strong validator for the gzip representation now labels identity bytes.
Proposed fix (apply manually; core publisher plus this bridge): when RC reports a decoded response, negotiate the client encoding from the reader (negotiate_reader_compression and set_response_compression already exist), weaken or drop the origin ETag for the re-encoded representation, and add Vary: Accept-Encoding. Alternatively ask EdgeZero for an encoded-passthrough response mode. A native test with a gzip origin should assert the client receives an encoded body and no reused strong ETag.
| ))); | ||
| } | ||
| if let Some(state) = state.as_ref() { | ||
| app.set_response_egress_observer(state.delivery_observation.clone()); |
There was a problem hiding this comment.
❓ question: Is a 30 s response-write cutoff on every routed response intended?
No TS adapter installs a response egress policy, so EdgeZero's default_response_egress_policy applies everywhere: write_deadline = egress_started_at + DEFAULT_RESPONSE_WRITE_BUDGET (30 s, edgezero-core response_egress.rs:1090), enforced per chunk in Fastly's pump_body/ensure_deadline after the response is committed. Base streamed with no deadline of its own.
Large downloads (PDF, zip, video) and SSE or long-poll endpoints through publisher passthrough will be cut off at 30 s on Fastly, and the same default applies on the other adapters. The spec says Fastly must preserve current behavior. If this is intended, please document it; if not, install a longer or route-specific budget for streamed routes with set_response_egress_policy here.
| SourceFailure, | ||
| } | ||
|
|
||
| fn router(source_failure: bool) -> RouterService { |
There was a problem hiding this comment.
🔧 wrench: These tests don't exercise TS's own delivery gate, finalizer or observer wiring
delivery_tests.rs builds a toy router, a writer that re-implements RC state transitions by hand, and a finalize stub that only calls apply_terminal_response_effects. Nothing references handle_request, edgezero_main or build_app_with_state. Each of these mutations survives both test-fastly and test-fastly-reuse:
- deleting the
.is_some_and(|state| state.delivery_observation.completed())guard atmain.rs:541-543, which lets pull sync run after a fallback or failed delivery; - deleting
app.set_response_egress_observer(..)atapp.rs:1324, which silently disables pull sync in production; - reordering the finalizer steps at
main.rs:499-536; - dropping the platform-replacement check at
app.rs:1318-1322.
The spec's Fastly proof list (finalization order, post-send gating, failed-build retry, probe laziness, default vs reusable modes) is therefore not met; both aliases run the same mode-agnostic tests.
Proposed fix (apply manually; small refactor plus tests): extract the pull-sync gate and the finalizer closure in main.rs into functions and test them directly, and add a test that build_app_with_state installs the observer, for example by dispatching through the built app and asserting delivery_observation.completed() flips. Smaller: several assertions here have no message (:97-110, :122-127, :164-179).
| return new Promise((resolve, reject) => { | ||
| const req = http.request( | ||
| `${endpoint}${path}`, | ||
| { method, headers }, |
There was a problem hiding this comment.
⛏ nitpick: Enforce the "no environment proxy" intent
With NODE_USE_ENV_PROXY=1 and HTTP_PROXY set, Node 24 routes these loopback requests through the proxy (probe: ECONNREFUSED at the proxy). agent: false bypasses it.
| { method, headers }, | |
| { method, headers, agent: false }, |
| clippy-cloudflare-wasm = "clippy -p trusted-server-adapter-cloudflare --target wasm32-unknown-unknown --features cloudflare --lib -- -D warnings" | ||
| test-cloudflare = "test -p trusted-server-adapter-cloudflare" | ||
|
|
||
| # --- Spin adapter (native host tests + wasm32-wasip1 target) --- |
There was a problem hiding this comment.
⛏ nitpick: Stale target in the section comment; the aliases below use wasm32-wasip2.
| # --- Spin adapter (native host tests + wasm32-wasip1 target) --- | |
| # --- Spin adapter (native host tests + wasm32-wasip2 target) --- |
| /// with EID arrays) while preventing an authenticated client from consuming | ||
| /// arbitrary WASM linear memory. | ||
| const MAX_AUCTION_BODY_SIZE: usize = 256 * 1024; | ||
| /// Maximum request bytes buffered by adapters for the auction endpoint. |
There was a problem hiding this comment.
⛏ nitpick: The new sentence was appended to the existing summary paragraph. Keep the one-sentence summary, then a blank line and the adapter detail.
| /// Maximum request bytes buffered by adapters for the auction endpoint. | |
| /// | |
| /// Adapters buffer at most this many request bytes before calling [`handle_auction`]. |
| // that did not itself validate a credential. Without this, a request marked | ||
| // upstream and then routed to an unprotected path would keep an assertion | ||
| // nothing checked. | ||
| req.extensions_mut().remove::<EdgeTerminatedAuthorization>(); |
There was a problem hiding this comment.
🏕 camp site: The refactor dropped the security rationale for clearing the marker
The removed comment explained why this must happen before any early return. Restoring it keeps that reasoning next to the code that depends on it. The same refactor also dropped the note above the Authorized marker ("so the shared template cache can distinguish it from a credential meant for the origin"), which would be worth restoring above AuthDecision::Authorized near line 155.
| req.extensions_mut().remove::<EdgeTerminatedAuthorization>(); | |
| // Cleared before any early return so no inherited marker can survive a call | |
| // that did not itself validate a credential. Without this, a request marked | |
| // upstream and then routed to an unprotected path would keep an assertion | |
| // nothing checked. | |
| req.extensions_mut().remove::<EdgeTerminatedAuthorization>(); |
| let ec_state = response.extensions_mut().remove::<EcFinalizeState>(); | ||
| let asset_cache_policy = response.extensions_mut().remove::<AssetProxyCachePolicy>(); | ||
| let request_filter_effects = response.extensions_mut().remove::<RequestFilterEffects>(); | ||
| let settings = settings_snapshot.clone().or_else(|| { |
There was a problem hiding this comment.
⛏ nitpick: Fastly finalize and log cleanups
.ok()here drops the twolog::warn!s base had ("entry-point finalize skipped…", "EC finalize skipped…"), so a settings reload failure is now silent; log before.ok().- Post-commit failure logs (
main.rs:551) lost the[instance= ordinal= request=]counter_contexttags, while the doc atmain.rs:244-247still says such failures are "reconciled during analysis". - Docs still describe TS-owned sending:
main.rs:108-110("returns()"),main.rs:169-172("must send exactly once", thoughErrpaths now send nothing), plusmain.rs:86-89and:244-247.
…tity Keep the public PR 1506 documentation closeout, current-main public-origin and version guidance, and operator-versus-live-runtime schema behavior. Update active pin descriptions to the already-qualified EdgeZero prerelease. The only Rust change in the closeout is documentation on the existing Fastly error helper.
Preserve main 06e51be through an ordinary merge. Keep strict operator integration IDs/fields, the intended live-runtime warning policy and the existing framing/TLS/readthrough compatibility distinction. Adapt mediator error fallback and its telemetry assertions to direct bounded EdgeZero calls. HTTP failures and provider Error statuses rank collected bids locally with floors; valid no-bid and 204 remain final mediation decisions. Exercise both ordinary and split dispatch/collect paths, without restoring old HTTP/backend or executor bridges.
The qualification target is intentionally outside the checkout. Ask Cargo for its actual output root rather than executing binaries under a hardcoded ignored target directory. Preserve all cache and purge assertions.
There was a problem hiding this comment.
Superseded by a follow-up review with inline comments: #1531 (review)
dhruv8sh
left a comment
There was a problem hiding this comment.
Summary
This PR moves the Axum adapter to strict prepared-store startup. It verifies the config envelope, collects the active secret keys, reads them within limits and builds immutable snapshots before the listener binds. The legacy TRUSTED_SERVER_CONFIG_*/TRUSTED_SERVER_SECRET_* env stores and PORT are removed. The core refactor in config_payload.rs and secret_resolution.rs is clean and well tested. The main issue is that the primary onboarding guide still uses the removed inputs.
1 of the inline comments below carries a one-click GitHub
suggestion. Use Commit suggestion to apply it. The other comment is a question and has no code change.
Blocking
🔧 wrench
- Getting-started guide still uses removed Axum inputs — see Cross-cutting below
Non-blocking
🤔 thinking / ⛏ nitpick
- DataDome payload falls back to raw-target evidence when checked ingress cannot be resolved — see inline at
crates/trusted-server-core/src/integrations/datadome/protection.rs:250 reserve_portdoes not hold a reservation — see inline atcrates/trusted-server-adapter-axum/tests/native_acceptance.rs:280
Cross-cutting / body-level findings
- 🔧 Getting-started guide still uses removed Axum inputs —
docs/guide/getting-started.md:86-125(not in this diff) still tells users to exportTRUSTED_SERVER_CONFIG_TRUSTED_SERVER_CONFIG_TRUSTED_SERVER_CONFIGandTRUSTED_SERVER_SECRET_TRUSTED_SERVER_SECRETS_*, to setPORT=<port>, and it documents theTRUSTED_SERVER_{CONFIG,SECRET}_{STORE}_{KEY}convention table. This PR removes the env-backed stores (platform.rs) andport_from_env(main.rs).startup.rsreads secrets only from the preparedtrusted_server_secretsstore, andtests/startup.rsasserts that the legacy variables cannot rescue a missing input. A user who follows the quickstart now gets "a required app-config secret is missing" at startup, andPORTis ignored. Please update that section the same waydocs/guide/axum-dev.mdwas updated: useEDGEZERO__SECRETS__{HANDLER_PASSWORD,PUBLISHER_PROXY_SECRET,EC_PASSPHRASE}andEDGEZERO__ADAPTER__PORT, use the prepared local config map instead of exporting the envelope, and remove the convention table or replace it. Apply this manually, because the file is outside the diff.
CI Status
- worker-deadlines (spin): PASS
- worker-deadlines (cloudflare): PASS
- vitest: PASS
- ts dev lint domains (changed lines): PASS
- trace runtime and browser acceptance: PASS
- release scripts: PASS
- prepare integration artifacts: PASS
- integration tests (Fastly EC lifecycle): PASS
- integration tests: PASS
- HTML EOF runtime (Fastly): PASS
- format-typescript: PASS
- format-docs: PASS
- Fastly response diff script: PASS
- docs-proposal scripts: PASS
- CLAUDE.md symlink guard: PASS
- CI Node pin guard: PASS
- cargo test (ts CLI, native) (ubuntu-latest): PASS
- cargo test (ts CLI, native) (macos-latest): PASS
- cargo test (cross-adapter parity): PASS
- cargo test (axum native): PASS
- cargo test: PASS
- cargo fmt: PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo check/build/test (spin native + wasm32-wasip1): PASS
- browser integration tests: PASS
| .get::<edgezero_core::ingress::RequestIngress>() | ||
| .is_some() | ||
| { | ||
| RequestInfo::from_request(req, client_info).ok() |
There was a problem hiding this comment.
🤔 thinking — When RequestIngress is present but RequestInfo::from_request fails (for example an invalid Host), .ok() turns the error into None. The fallbacks below then use request_host(req) and req.uri().scheme_str(), which is the raw-target evidence this change tries to stop trusting. handle_first_party_proxy_sign handles the same case by propagating the error with ?. Should the DataDome payload also refuse the request, or send empty Host/Protocol fields, when ingress metadata exists but cannot be resolved? Then the two paths would behave the same way.
| } | ||
|
|
||
| fn reserve_port() -> tokio::net::TcpSocket { | ||
| // A listening reservation can satisfy another test's negative startup probe. |
There was a problem hiding this comment.
⛏ nitpick — spawn reads the port from a temporary (reserve_port().local_addr()…port()), so the socket is dropped before the child binds. That is necessary, because a bound socket would make the child's bind fail, but it means nothing is actually reserved. A parallel test can still receive the same ephemeral port. The comment should describe what actually happens:
| // A listening reservation can satisfy another test's negative startup probe. | |
| // Bind without listening so another test's startup probe cannot connect. | |
| // The caller drops this socket after reading the port, so the child gets a | |
| // free ephemeral port, not a held reservation. |
Per the user, every feature is built once and works on every adapter in platform order, with platform gaps sent to EdgeZero (DEC-16, SH-5). T21 moves the identity routes, EC lifecycle, providers and write-back into one router in core that each adapter mounts; A2, A10, T8, T9, T17 and T18 shrink to turning it on per adapter and passing the conformance suite, and T7, T14 and A11 fold into A2 and A10. EZI-2a adds EdgeZero's local store for Axum development and EZI-4 binds Axum's geo provider from the manifest. Restack the Axum RC units on #1506: #1531 carries A1 in place of #1528, #1533 and #1534 close as copies of #1525 and #1526, and the image and Terraform units sit directly on #1532 so A6 and A7 stop waiting on A2. Record #1566, refresh the provider stack's fixes at their current heads, and flag #1535 as unplanned.
Per the user, the RC line is forward-ported to main (DEC-6). Rechecking the proposed stack against DEC-3 and DEC-16: #1536 and #1537 are mostly Axum-only stores, store selection and wiring that EZI-2b, T6 and T21 replace, so A2 becomes a new pull request that takes their tests, and both close with #1535 once their code moves. The image and Terraform units restack on #1543, which publishes the image and shares .dockerignore with #1539. #1531 goes on #1506 for A1 and #1532 on #1531 for #1529.
Per the user, binary attestation and package publishing (#1449, #1463, #1543) are not part of this work and carry lower priority (DEC-19). AX-5 keeps only the production image built from source, A6 shrinks to building it and no longer waits on #1449, and the image and Terraform units restack on #1531, where they apply cleanly, instead of #1543.
|
CREATOR: From the server-side identity plan (#1559, Part of #1573 |
Summary
Draft baseline review unit. Not a current-main merge candidate, deployment approval or completed current-head CI claim.
Baseline and sequence
Head:
b1216763a3c2a38df1a2909285330e818823423a. Base:baseline/demo-main-20261110.Historical main anchor:
2dd968aeebeb2cc0d5110115f64e0ad58e2e1d13. Original demo: TS2423a2ed plus separately preserved dirty inputs. Matching EdgeZero: draft431, exact revision83d7a6296d285e135c4ea70021b19e6c8dca7e9f.The user selected compatible-baseline cleanup. Current-main integration with EdgeZero462 and the authenticated-forwarder incompatibility are deferred. Preserve current-main PR1525/1526 ready and1527/1528 draft, with their existing bases. No excluded1499 or independent1506 implementation, relay/privacy experiment, new transport fallback, group/codec/phase design or operational deployment is included.
Changes
Closes
Refs #1530. This unit completes one part of the baseline extraction and does not close the overall tracker on its own.
Spec
Not a new feature. This is extraction of existing demo behavior and bounded build/caller corrections. Original native and EC storage design/review records remain in the baseline where applicable; they are not presented as approval of later combined source. No new forwarding, consent, timer or provider policy is designed here.
Test plan
The final b67 all-feature binary also passes pinned Moto and real AppConfig Agent fixtures: config-only, secret-only and combined health/auth/TSJS, immutable snapshots, restart adoption and selected-provider failure before listen without fallback. No external AWS account is contacted. CLI-export and DynamoDB are outside that harness.
Native startup and existing bounded-input/transport regressions pass. Core2990 pass; strict Core/Axum lint and formatting pass. Three additional tests delegate unset budgets to EdgeZero and preserve explicit deadlines without a TS30-second fallback.
The b67ffc6 baseline checkpoint has passing documented local gates: Fastly/default/reuse/native/Cloudflare/Spin suites, all adapter strict lint lanes, 17 cross-adapter parity cases, 1252 JS cases, CLI suites, formatting, clock/digest/proposal checks and changed-lines domain lint. Passing checks at121ebcc36 are reused only for unchanged relevant inputs; the final follow-up covers the CLI test-import and documentation-only corrections. These results are not a substitute for this PR's current-head hosted checks.
Checklist
Operational Terraform remains in trusted-server-deployer. Local checks and prototype tests do not qualify cloud access, fleet durability or a live publisher.
Hosted docs-build correction
The first hosted format-docs job caught an additional site-build gate that the original local formatter checkpoint had not run: a relative link from the native EC guide to a Terraform example outside the site. Native EC correction8b11dbad3 replaces it with the plain repository path; no dead-link check is disabled. That docs-only correction is carried through the existing upper PR identities. Native EC and final integration8985b111 both pass docs ESLint and VitePress build; the final docs formatter also passes. Runtime, dependency, image-build-context and test inputs are unchanged from b67ffc6, so their prior exact-input proofs are reused. Hosted current-head checks still control readiness.
Startup negative-probe correction
A hosted native EC case failed the no-listener probe while its child logged the expected startup validation failure. The old log has no socket endpoints, so the failure's exact subtype is not claimed confirmed. A separate controlled Linux reproduction demonstrates a successful TCP self-connection with no listening server. Native correction8490c691d distinguishes equal local/peer endpoints from a real listener; actual socket controls prove the self-connection is excluded and a real listener is still detected. Nonzero exit, expected diagnostic, secret-redaction and the original startup deadline remain required. This changes only the acceptance fixture, not production admission, transport or trust.
The earliest native unit and final9356c557 default/all-feature acceptance checks and strict lint pass, including both controls and the original failure case. Ignores remain explicit; earlier separately executed celld proof is not relabeled as a new ignored-case run. Correction is merged upward through existing PR identities without force-pushing or retargeting. Current-head hosted checks must pass; the old failure is retained rather than rerun-only waived. Runtime/API/dependency inputs remain unchanged. A fresh exact9356 producer build and real runtime smoke now pass: image
sha256:65a735d38e9dbc0573d0f6068a8b261bd202c43aea05085c2b2d6bccc91ebdbd, linux/amd64 UID/GID10001, including startup rejection and in-flight SIGTERM drain. Prior b67 image receipts remain separate; no image was pushed. All local parent runners are complete.Browser CI host compatibility
Preserved older browser failures show Playwright1.58.2 rejecting ubuntu-latest's new Ubuntu26.04 image before browser installation; later missing-executable failures are consequences, not passing browser tests. Nativeb1216763a pins only the browser job to ubuntu-24.04, leaving dependency versions, tests and assertions unchanged. Direct Actionlint1.7.12 passes at the owning native and final integration points; the first shim-only attempt could not run because no global version was configured, and is not counted as lint proof. No global tooling was changed. The only9356 ->ee7 delta is this workflow host/comment; runtime, Cargo, docs, tests and Docker-context inputs are unchanged. Actual browser execution on the supported hosted runner remains pending and must pass before readiness. No waiver or browser skip.