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-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)

[aram356]

Summary

Round-6 review (worktree). The handler-layer migration to http::{Request,Response}<EdgeBody> is structurally sound, the round-5 blocking findings (streaming regression, contradictory debug_assert! vs test, multi-value header preservation, 413 regression tests, # Panics doc mismatch) are all resolved, and cargo test --workspace (1016 tests), cargo clippy --workspace --all-targets --all-features -- -D warnings, and cargo fmt --all -- --check pass locally.

Two new correctness regressions block merge:

1. The streaming_body.finish() fix in commit cd4c621 silently completes truncated responses on mid-stream errors — a regression from the prior drop(streaming_body) semantics that the original adapter comment explicitly relied on for client-visible truncation.
2. The outcome.status() == UNAUTHORIZED gate strips geo headers from origin-forwarded 401s, conflating two distinct sources of 401 and losing observability signal without addressing the original threat model.

Both are introduced by this PR's restructuring of the adapter entry and the new streaming dispatch arm. Neither was caught by prior rounds because the round-5 review concluded before commit cd4c621 (the streaming fix) landed, and the geo-status gate was added in this PR's rewrite of route_request.

A handful of non-blocking concerns and carry-overs from rounds 3/4 round out the review.

Blocking

🔧 wrench

• streaming_body.finish() on error masks truncation: previously the error path dropped the streaming body so the client saw an EOF (truncated response). Commit cd4c621 made it call finish() in both arms, cleanly closing chunked encoding even on mid-pipeline failures. Client now sees a successful-looking partial response. See inline at main.rs:188.
• outcome.status() == UNAUTHORIZED conflates our 401 with origin-forwarded 401: geo is stripped from any response that happens to be 401, not just our enforce_basic_auth challenge. See inline at main.rs:146.

Non-blocking

🤔 thinking

• stream_publisher_body doc claims async but the function is sync (publisher.rs:402-405). The async/block_on story applies to handle_publisher_request, not this function. See inline.
• to_fastly_request_ref silently drops body (compat.rs:102). Audited safe today, but no compile-time signal. See inline.

♻️ refactor

• Body-size cap pattern duplicated 6× (carry-over from rounds 3 & 4). proxy.rs:886, :1009, auction/endpoints.rs:45, 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. See inline.

🌱 seedling

• DEFAULT_PUBLISHER_FIRST_BYTE_TIMEOUT = 15s still hardcoded (publisher.rs:21). Carry-over from round 1.
• Body-size caps still fire after req.take_body_bytes() materialises (compat.rs:51). Carry-over from round 3 — receive-side allocation unprotected.

📝 note

• No test covering to_fastly_response_skeleton (compat.rs:167). The function is reachable from a single call site (the streaming dispatch in main.rs:169). A test that round-trips status + headers (minus body) would protect the streaming-dispatch contract. Not blocking — the function is short and stream_publisher_body_* tests indirectly exercise the streaming path.

Resolved from prior rounds

• ✅ Round-5 streaming regression — HandlerOutcome::Streaming + to_fastly_response_skeleton + stream_publisher_body restores the dispatch (main.rs:163-191, compat.rs:167)
• ✅ Round-5 cargo test --workspace failure — debug_assert removed from to_fastly_response (compat.rs:148-153)
• ✅ Round-4 doc/code mismatch on # Panics — compat.rs:45-49 and :61-65 now match the URL-parse-with-fallback behaviour
• ✅ Round-4 missing 413 tests on /verify-signature, /admin/keys/*, /auction — request_signing/endpoints.rs:926, 944, 962, auction/endpoints.rs:151
• ✅ Round-3 to_fastly_request_ref multi-value header bug — compat.rs:105 uses append_header, regression test at compat.rs:509
• ✅ Round-3 platform_response_to_fastly duplicated into orchestrator.rs — now lives only in compat.rs:276, returns Err on streaming
• ✅ Round-1 substring matching in migration_guards.rs — now uses \b…\b regex with comment stripping (migration_guards.rs:48-51)
• ✅ Round-1 geo-before-auth concern — geo lookup now runs after routing and is skipped for UNAUTHORIZED outcomes (main.rs:145-156; though see the new 🔧 finding above on the gate's overreach)

CI Status

• fmt: PASS (locally)
• clippy: PASS (locally)
• rust tests: PASS (locally, 1016 tests)
• js tests: PASS (CI)
• integration tests: PASS (CI)
• browser integration tests: PASS (CI)