Migrate handler layer to HTTP types (PR 12)

[prk-Jr]

Summary

• Move the PR 12 handler boundary to http request/response types so core handlers no longer depend on Fastly request/response APIs.
• Keep Fastly isolated to the adapter entry/exit path and the still-unmigrated integration/provider boundary, which keeps PR 13 scope separate.
• Lock the migrated surface with tests and migration guards so later changes do not accidentally reintroduce handler-layer Fastly types.

Changes

File	Change
crates/trusted-server-adapter-fastly/src/main.rs	Move the conversion boundary to the adapter entry/exit path and route core handlers with HTTP-native requests and responses.
crates/trusted-server-core/src/auction/endpoints.rs	Migrate the /auction handler to http types and rebuild a Fastly request only at the provider-facing AuctionContext boundary.
crates/trusted-server-core/src/auction/formats.rs	Consume HTTP requests directly and return HTTP auction responses.
crates/trusted-server-core/src/auction/orchestrator.rs	Keep provider parsing Fastly-based while making the provider response conversion explicit in the orchestrator.
crates/trusted-server-core/src/compat.rs	Add the minimal borrowed HTTP-to-Fastly request bridge needed for the remaining provider boundary.
crates/trusted-server-core/src/geo.rs	Make response header injection operate on HTTP responses.
crates/trusted-server-core/src/integrations/google_tag_manager.rs	Adapt the integration boundary so the shared proxy helper can stay HTTP-native while the integration trait remains Fastly-based.
crates/trusted-server-core/src/integrations/gpt.rs	Adapt the integration boundary so the shared proxy helper can stay HTTP-native while the integration trait remains Fastly-based.
crates/trusted-server-core/src/integrations/testlight.rs	Adapt the integration boundary so the shared proxy helper can stay HTTP-native while the integration trait remains Fastly-based.
crates/trusted-server-core/src/migration_guards.rs	Extend the guard to cover the handler modules migrated in PR 12.
crates/trusted-server-core/src/proxy.rs	Migrate first-party proxy/click/sign/rebuild handlers and response rewriting to HTTP request/response types.
crates/trusted-server-core/src/publisher.rs	Migrate publisher handlers to HTTP types and use the platform HTTP client for publisher-origin fetches.
crates/trusted-server-core/src/request_signing/endpoints.rs	Migrate request-signing and admin handlers to HTTP request/response types.

Closes

Closes #493

Test plan

• cargo test --workspace
• cargo clippy --workspace --all-targets --all-features -- -D warnings
• cargo fmt --all -- --check
• JS tests: cd crates/js/lib && npx vitest run (not run; no JS/TS files changed)
• JS format: cd crates/js/lib && npm run format
• Docs format: cd docs && npm run format
• WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
• Manual testing via fastly compute serve
• Other: verified remaining Fastly usage is limited to the adapter boundary and the still-unmigrated integration/provider boundary

Checklist

• Changes follow CLAUDE.md conventions
• No unwrap() in production code — use expect("should ...")
• Uses tracing macros (not println!)
• New code has tests
• No secrets or credentials committed

[aram356]

Summary

Round 3 review. The PR cleanly addresses every blocking finding from the prior two rounds: /verify-signature and /auction now enforce body-size caps (4 KiB and 64 KiB), platform_response_to_fastly was deleted from auction/orchestrator.rs (both call sites use crate::compat::to_fastly_response directly), and /admin/keys/{rotate,deactivate} got matching 4 KiB caps. The expect() in finalize_response (main.rs:251-253) is now safe because Settings::prepare_runtime validates response_headers at startup (settings.rs:486-498).

One new blocking finding: to_fastly_request_ref (introduced in PR 12) silently collapses multi-value headers, which is a latent correctness regression in the provider path. See inline.

CI is green; local cargo test --workspace (814 passed), cargo clippy --workspace --all-targets --all-features -- -D warnings, and cargo fmt --all -- --check all pass.

Blocking

🔧 wrench

• to_fastly_request_ref clobbers multi-value headers: see inline at compat.rs:90.

Non-blocking

🤔 thinking

• VERIFY_MAX_BODY_BYTES = 4096 may be tight for legitimate signed payloads (request_signing/endpoints.rs:78). After base64 signature (~88 bytes for Ed25519), kid, JSON wrapper, and quoting/escaping, the usable payload budget is ~3 KiB. Plausibly fine for typical OpenRTB-shaped payloads but can clash with publishers signing larger blobs (e.g. embedded events). Consider a brief // safety: comment explaining the chosen limit, or making it config-derived alongside the existing GTM max_beacon_body_size pattern.

📝 note

• Body-size caps fire after the body is already materialized into memory (compat.rs:40-43). compat::from_fastly_request calls req.take_body_bytes() at the adapter entry, so the entire body is buffered before any handler-level cap runs. The new caps protect parse allocations but not the receive allocation. This matches the compat shim's semantics today and is a PR-15 concern, not PR-12 — worth pinning in the PR 15 plan that streaming-aware caps need to land with the boundary removal so the protection isn't quietly downgraded when the shim is replaced.

♻️ refactor

• No 413 regression tests for /verify-signature, /auction, /admin/keys/{rotate,deactivate} (request_signing/endpoints.rs:93,183,292, auction/endpoints.rs:45). This commit added sign_rejects_oversized_body and rebuild_rejects_oversized_body (proxy.rs:2358-2395) — exactly what the previous review asked for. The same pattern wasn't applied to the four endpoints whose caps are introduced in this commit. Without negative tests, a future commit that drops any cap won't be caught. Same shape as the existing proxy tests — five lines per endpoint.

• Body-size-cap pattern duplicated 6× across handlers. if body.len() > MAX { return Err(Report::new(RequestTooLarge { format!("X payload {} exceeds limit of {}", body.len(), MAX) })); } appears at proxy.rs:884, proxy.rs:1007, auction/endpoints.rs:45, and three times in request_signing/endpoints.rs (93, 183, 292). A small helper (fn enforce_max_body_size(bytes: &[u8], limit: usize, what: &'static str) -> Result<(), Report<TrustedServerError>>) in http_util would drop ~50 lines and centralize the message format. Defer if you'd rather land in a follow-up.

CI Status

• browser integration tests: PASS
• integration tests: PASS
• prepare integration artifacts: PASS

[aram356]

Summary

Round-4 review. Round-3's blocking finding (to_fastly_request_ref clobbering multi-value headers) is resolved at compat.rs:95 with a regression test at compat.rs:478. CI is green.

Two new concerns this round: (1) the round-3 ♻️ ask for 413 regression tests on the four endpoints whose caps were introduced in this PR is still unaddressed — only the proxy endpoints got sign_rejects_oversized_body/rebuild_rejects_oversized_body. (2) Two doc-vs-code mismatches in compat.rs — the # Panics blocks describe a panic site that doesn't exist (URL parsing has a silent fallback to /), while the real panic site (header construction) is undocumented.

Blocking

🔧 wrench

• No 413 regression tests for /verify-signature, /admin/keys/{rotate,deactivate} (request_signing/endpoints.rs:103, :254, :381). Round-3 explicitly asked. See inline.
• No 413 regression test for /auction; file has no #[cfg(test)] module (auction/endpoints.rs:45). See inline.

Non-blocking

🤔 thinking

• # Panics doc claims URL-parse panic that cannot occur (compat.rs:42-44, compat.rs:56-58). Doc/code mismatch. See inline.
• Silent URL fallback to / masks malformed URIs (compat.rs:18-21). build_http_request uses unwrap_or_else(|_| http::Uri::from_static("/")). Realistically unreachable given Fastly pre-validation, but produces silent misrouting (catch-all hits the publisher origin at main.rs:215) if it ever fires. Either document why the fallback is reachable-but-safe, or assert the invariant with expect("should parse Fastly-validated URL").

♻️ refactor

• 65536 literal duplicated 4× in proxy.rs (proxy.rs:884, :887, :1007, :1010) — the only handler in this PR not using named constants. See inline.
• String::from_utf8(body_bytes.to_vec()) allocates twice (proxy.rs:892, :1015). See inline.
• Body-size cap pattern duplicated 6× (carry-over from round 3). Same if body.len() > MAX { return Err(RequestTooLarge { format!(...) }) } shape at proxy.rs:884, proxy.rs:1007, auction/endpoints.rs:45, and three times in request_signing/endpoints.rs (103, 254, 381). A small enforce_max_body_size(bytes, limit, what) helper in http_util would centralize the message format and remove ~50 lines.

🌱 seedling

• DEFAULT_PUBLISHER_FIRST_BYTE_TIMEOUT = 15s still hardcoded (publisher.rs:21). Carry-over from round 1 — worth a config knob during PR-15 boundary cleanup.
• Body-size caps still fire after req.take_body_bytes() materializes (compat.rs:46). Carry-over from round 3 — caps protect parse allocations but not the receive allocation. PR-15 streaming-aware shim should land alongside boundary removal so the protection isn't quietly downgraded.

📝 note

• platform_response_to_fastly is now the only diverging conversion (compat.rs:245-264). Returns Err on streaming bodies instead of to_fastly_response's silent-drop. The asymmetry is correct for orchestrator use, but worth pinning in the PR-15 plan so streaming closes consistently.

CI Status

• browser integration tests: PASS
• integration tests: PASS
• prepare integration artifacts: PASS

[aram356]

Summary

Re-review of the handler-layer HTTP-types migration. The migration is structurally clean, but two concerns block merge: (1) the publisher streaming dispatch added in #562 is regressed — PublisherResponse::Stream now buffers the entire pipeline output instead of writing chunks via stream_to_client(), and (2) cargo test --workspace fails on the current head because the debug_assert! in compat::to_fastly_response (introduced in PR 11 and inherited here) contradicts the test that exercises the documented silent-drop fallback. CI did not catch (2) because PR 624's base branch causes test.yml and format.yml to skip.

Blocking

🔧 wrench

• Streaming regression on the publisher fallback path. Before this PR, Ok(PublisherResponse::Stream { … }) in adapter-fastly/src/main.rs called response.stream_to_client() and wrote chunks via stream_publisher_body. After this PR, resolve_publisher_response materializes the entire pipeline output into a Vec<u8>, sets Content-Length, replaces the body, and the adapter sends via compat::to_fastly_response(response).send_to_client(). The whole publisher response is now buffered in WASM memory.

  Observable: the PublisherResponse::Stream enum variant becomes equivalent to PublisherResponse::Buffered until streaming is restored. TTFB is delayed by the full pipeline pass, and WASM memory pressure scales with body size.

  Structural cause: compat::to_fastly_response cannot move an EdgeBody::Stream into a fastly::StreamingBody — there is no streaming bridge across the compat shim. Two reasonable directions:

  • Preferred: Keep the streaming dispatch in the adapter. Have route_request return Result<HandlerOutcome, …> where HandlerOutcome::Streaming(…) carries the unbuffered body+params, and main() opens stream_to_client() on the converted Fastly response and calls stream_publisher_body against it. Honest "no behavior regressions" version of PR 12.
  • Alternative: Block the streaming arm and route everything through BufferedProcessed until PR 15 introduces a streaming body bridge — but explicitly call this out as a known regression in the PR description and add a tracking issue.

  No test today asserts streaming behavior. The publisher tests (stream_publisher_body_*) all use Vec<u8> outputs and don't distinguish streamed-to-client from buffered-then-sent. So the regression is entirely silent.

• cargo test --workspace fails: compat::tests::to_fastly_response_with_streaming_body_produces_empty_body panics on a debug_assert! that contradicts the test.

  In crates/trusted-server-core/src/compat.rs (line ~138), the function asserts matches!(&body, EdgeBody::Once(_)). The test below it (line ~676) constructs an EdgeBody::Stream and asserts the body comes out empty — so the test panics in cargo test debug builds.

  Both the debug_assert! and the test were introduced in PR 11 (commit 76df6f8), so PR 624 inherits the contradiction rather than introducing it — but it materializes here because (a) the PR's test plan checkbox [x] cargo test --workspace does not pass on the current head, and (b) when this stack lands on main and a downstream PR opens, the standard test.yml workflow will fail. The PR 11 review can fix it upstream, or this PR can apply the fix.

  Fix (preferred — the warn-and-empty fallback is documented behavior):

  pub fn to_fastly_response(resp: http::Response<EdgeBody>) -> fastly::Response {
      // … build headers …
      match body {
          EdgeBody::Once(bytes) => {
              if !bytes.is_empty() {
                  fastly_resp.set_body(bytes.to_vec());
              }
          }
          EdgeBody::Stream(_) => {
              log::warn!("streaming body in compat::to_fastly_response; body will be empty");
          }
      }
      fastly_resp
  }

  Alternative: keep the debug_assert! and gate the test on #[cfg(not(debug_assertions))]. But the warn-log already covers misuse and the documented fallback path should be exercised by tests.

Non-blocking

🤔 thinking

• Silent body drop on EdgeBody::Stream in compat. Both to_fastly_request and to_fastly_response log a warning and emit an empty body when handed a streaming EdgeBody. Combined with the streaming-dispatch removal above, this means a future caller that tries to stream through the compat boundary will silently see truncated bodies. Promote the misuse to a Result return (the Fastly adapter's edge_request_to_fastly already does this), or add a comment listing the audited non-streaming call sites so the constraint is verifiable. Not in PR 12's diff but the streaming-regression fix will reintroduce the constraint.

♻️ refactor

• bytes.to_vec() copies the full body across the compat boundary. compat.rs:144-146 and compat.rs:81-84 memcpy the body. For large publisher responses this is unavoidable O(N) at the shim. Worth a one-line comment that this cost goes away with PR 15 so future readers don't try to optimize inside the shim. (Not in PR 12's diff.)

🌱 seedling

• Sync between route_request admin routes and Settings::ADMIN_ENDPOINTS is comment-only. main.rs:166 carries a "Keep in sync with Settings::ADMIN_ENDPOINTS" comment. A test that compares the constant against the routes registered in route_request would prevent future drift. Follow-up issue.

📝 note

• CI workflow gate gap. .github/workflows/test.yml and .github/workflows/format.yml trigger only on pull_request: branches: [main]. PR 624 targets the PR 11 feature branch, so cargo test, fmt, and clippy never ran on this PR. Only integration-tests.yml ran (and passed). This is how the failing compat test stayed invisible. Out of scope for PR 12 to fix — but the consequence (a regression that will only surface when this lands on main and a downstream PR opens) is in scope. Either drop the branches: [main] filter for the EdgeZero stack PRs, or workflow_dispatch the test workflow against each stack head.

CI Status

• fmt: PASS (locally)
• clippy: PASS (locally)
• rust tests: FAIL locally (compat::tests::to_fastly_response_with_streaming_body_produces_empty_body); not run on CI for this PR
• integration tests: PASS (CI)
• browser integration tests: PASS (CI)