[prk-Jr]

Summary

Add a default-off, same-tab mobile ad-rendering trace workflow with explicit Enable/End controls, bounded browser-local capture, a consolidated viewer and JSON export. Preserve ordinary advertising when transport, correlation, storage or browser APIs are unavailable.

• Authenticate and handle reserved trace paths before ordinary middleware across Fastly, Axum, Cloudflare and Spin. Freeze request-local cookie/session gates and runtime ingress evidence.
• Carry bounded, redacted server auction evidence alongside distinct browser and creative observations, with opaque slot/cycle correlation.
• Serve hardened setup/viewer assets, validate the versioned report schema, and support Copy/Download/Share with independent local cleanup and server session termination.
• Authenticate public-origin forwarding separately from transport ingress and client-IP trust. Strip credentials before routing. The CLI companion is Preserve browser Origin with authenticated proxy forwarding #1251.

Review corrections

Both setup and traced publisher-document metadata use the authenticated reader IP captured before header sanitation. Runtime geo is omitted when it describes a different address. Ordinary request handling on Axum, Cloudflare and Spin retains its existing runtime client address.

Normal builds compare rebuilt trace assets with committed bytes. Prebuilt builds additionally require matching source provenance. The digest covers dedicated trace inputs and the locked Vite dependency graph, excludes the Node pin, and ignores publisher-only dependencies. Trace CSS builds disable ambient PostCSS discovery. CI now builds into a temporary bundle directory and runs TSJS_PREBUILT_DIR=<dir> cargo check-axum --locked, exercising the Rust source digest alongside the JS manifest checks. The published guide links to the asset-versioning follow-up on main. Asset provenance matches the checked-in trace sources and locked Vite dependency graph.

A further review found that refreshed-slot summary cards could combine one request's timing with an earlier request's auction. Auction links now match both slot and representative request number. Two regression cases reproduce the mismatch before the fix and pass afterward. The unpublished v1 JS asset and manifest were regenerated for this correction; the CSS bytes are unchanged. Published asset URLs must still receive a new version instead of replacement bytes.

Runtime path normalization remains adapter-dependent: aliases still visible at dispatch are rejected; Cloudflare may normalize an alias to an exact route before the hook.

The branch is current with main through 054c7b777. Changelog conflicts preserve the trace/forwarder security contract alongside main's config-permission and independent click-rewriting notes. The configuration table describes trusted_client_ip.shared_secret as a secret-store reference, matching runtime loading.

Released EdgeZero dependency

All six workspace EdgeZero dependencies now select the released v0.0.11 tag; all eight lockfile entries resolve to 87456c708ffe9edf86b3a0f8b06dfd15d60b07b5. EdgeZero #403 was merged and released on 2026-10-28.

The migration uses edgezero_core::ingress and the released shared-hook and three-argument ingress APIs. Fastly converts directly through public into_core_request and consumes the ingress metadata captured by EdgeZero. Earlier native sanitation leaves the client-request marker, URL and Host unchanged, so no application-owned snapshot override is needed. Trace-path classification still runs before native shortcuts. The obsolete helper-only test is removed, and all three implementation-plan instructions now describe adapter-owned capture.

Validation

Latest correction, signed and GitHub-verified commit 1464ab60c: all 33 local verification groups passed using Rust 1.98.1, Node 24.20.0 and Viceroy 0.21.0. Coverage includes the adapter lint/test matrix, CLI/codegen tests, parity, native build-digest tests/lint, WASM clock lint, JS build/tests/lint/format, docs/Markdown/proposal checks, a fresh production Fastly WASM build and the actual trace_runtime_boundary_fastly probe. All 25 hosted checks passed on this exact revision, including trace runtime/browser acceptance.

Earlier combined-feature validation follows, tied to its recorded revision:

For signed, GitHub-verified commit 74c6d3b49, which integrates main through 054c7b777:

• All 25 reported hosted checks passed on this exact revision, including general integration, browser integration, trace-runtime/browser acceptance, Fastly EC lifecycle and the platform matrix. GitHub's synthetic merge has the same tree as the locally verified commit.
• All 39 local verification groups passed: the adapter lint/test matrix, CLI and codegen tests, parity tests/lint, native build-digest tests/lint, WASM clock lint, both emitted-script regressions, prebuilt embedding, JS build/lint/format/tests and docs/Markdown/proposal checks.
• Actual Axum and Fastly EOF controls passed; Fastly also covered streaming in a fresh release fixture build. Fixture builds enable test-utils. Tools matched the repository pins: Rust 1.98.1, Node 24.20.0 and Viceroy 0.21.0.
• The managed staged-domain hook passed without allowlist additions or suppressions. Inherited malformed-URL and lookalike-host fixtures now use reserved example domains. Temporarily removing the hostname dot-boundary guard fails the retained lookalike-host test; the production guard was restored before verification.

Trace remains disabled by default. Physical iOS Safari/Android Chrome, browser session restoration, deployed HTTPS cookie workflows and operator staging/privacy/CDN/CSP acceptance remain release gates. Local automated checks do not establish those results.

Closes #1050
Closes #1108

[jevansnyc]

Review of the design spec. No code changes here, so this is internal consistency plus whether the stated contracts hold against what is in the repo today (gpt_diagnostics.rs, publisher.rs, auth.rs, prebid_eids.rs, and the TS diagnostics store/types).

Seven issues inline. The first three change the design rather than the wording:

1. The ts-auc- token shape does not match the existing producer, so correlation never joins.
2. TraceGptDiagnosticsV1 as specified cannot satisfy acceptance criterion 8.
3. Trace paths terminate ahead of authentication, which carves an exemption out of the ^/_ts namespace that auth.rs says should not exist.

The remaining four are bounded-scope corrections to the cookie lifetime claim, the TSJS gate, a capture_status gap, and redaction consistency.

Mechanical checks came back clean: cookie names (ts-ec, ts-eids, ts-tester, __Host-ts-console), the 8 KiB ts-eids cap (MAX_EIDS_COOKIE_BYTES), and the callback-issue reason values all match what the spec assumes.

[aram356]

Summary

A design-only PR adding a single 1852-line spec for a /_ts/trace mobile
diagnostics endpoint. The document is unusually well-grounded: field names,
constants, and several non-obvious hazards (the AuctionRequest.id leak, the
JA4/H2 fingerprint exclusion, bootstrap-fallback argument tolerance, the
unpopulated asn) are verbatim correct against the code. All seven findings
from the previous round are genuinely resolved in e3f371f8c, and I re-verified
each rather than re-raising it.

The blocking findings below are places where the spec mandates behavior the
pinned platform cannot express, or where it assumes adapter defaults that do the
opposite of what sections 8, 12.3, and 13 require. Because this is a design
document, each one is cheaper to fix now than after it becomes four
implementation PRs.

6 of the inline comments below carry a one-click GitHub suggestion —
use Commit suggestion (or Add suggestion to batch) to apply them. The
remaining comments describe the change in prose because the fix is a new
validation hook or spans more than one contiguous range.

Blocking

🔧 wrench

• Two-second request-body deadline is unimplementable — see inline at
  docs/superpowers/specs/2026-09-01-mobile-ad-render-trace-endpoint-design.md:494
• HEAD /_ts/trace is proxied to the publisher origin — see inline at
  docs/superpowers/specs/2026-09-01-mobile-ad-render-trace-endpoint-design.md:468
• Router-level 405 carries no Allow and no hardening headers — see
  inline at
  docs/superpowers/specs/2026-09-01-mobile-ad-render-trace-endpoint-design.md:506
• The mandated configuration validation cannot fire — see
  Cross-cutting below

Non-blocking

🤔 thinking / 📝 note

• AuctionSlot.ext presented as an existing type — see inline at
  docs/superpowers/specs/2026-09-01-mobile-ad-render-trace-endpoint-design.md:908
• Slot tokens described as existing; verbatim-comparison rule conflicts with
  normalizedAuctionId — see inline at
  docs/superpowers/specs/2026-09-01-mobile-ad-render-trace-endpoint-design.md:819
• Inherited and new cookie caps are presented as one list — see inline
  at
  docs/superpowers/specs/2026-09-01-mobile-ad-render-trace-endpoint-design.md:654
• CSP blocks the favicon, and leaves blob: and inline styles unaddressed
  — see inline at
  docs/superpowers/specs/2026-09-01-mobile-ad-render-trace-endpoint-design.md:1326
• Auth contract overstates rule composition; existing JA4 route is an
  auth-bypass precedent — see inline at
  docs/superpowers/specs/2026-09-01-mobile-ad-render-trace-endpoint-design.md:1364
• Container-nesting cap has zero headroom — see inline at
  docs/superpowers/specs/2026-09-01-mobile-ad-render-trace-endpoint-design.md:1092
• source enum diverges from the existing AuctionSource — see inline
  at
  docs/superpowers/specs/2026-09-01-mobile-ad-render-trace-endpoint-design.md:841
• Deprecated /__ts/page-bids alias is uncovered — see inline at
  docs/superpowers/specs/2026-09-01-mobile-ad-render-trace-endpoint-design.md:996
• GPT projection prose is not a usable allowlist — see inline at
  docs/superpowers/specs/2026-09-01-mobile-ad-render-trace-endpoint-design.md:715

Cross-cutting / body-level findings

• 🔧 The configuration validation this spec mandates cannot fire as a
  validator rule. Section 8 (line 452) requires that
  “trace_page_enabled = true requires enabled = true; invalid
  combinations fail configuration validation”, and section 14.1 (line 1433)
  requires a test that configuration “rejects trace-page enablement without
  GPT diagnostics”.

  One correction first, in fairness to the design: adding trace_page_enabled
  to TOML today is already a hard error on both the enabled and disabled
  paths, because validate_disabled_schema
  (crates/trusted-server-core/src/settings.rs:247-253) forgives only errors
  beginning "missing field ", and an unknown-field error is not forgiven.
  That part is correctly fail-closed.

  The real gap is narrower but still real. Once trace_page_enabled is a
  legitimate field, IntegrationSettings::get_typed returns before validation:

  // crates/trusted-server-core/src/settings.rs:348-350
  if !config.is_enabled() {
      return Ok(None);
  }
  
  config.validate().map_err(|err| { /* :352 */ })?;

  So the exact combination the spec wants rejected — enabled omitted (serde
  default false) together with trace_page_enabled = true — resolves to
  Ok(None) and a #[validate(schema(...))] rule never runs. Note
  #[validate(schema(...))] does otherwise work in this crate
  (settings.rs:2737), so the failure mode is silent rather than obvious.

  The precedent that fits is validate_js_asset_proxy_config
  (crates/trusted-server-core/src/config.rs:280-299): it reads the raw JSON
  and calls validate() outside the enabled gate, runs from both the deploy
  (config.rs:250) and runtime (config.rs:271) paths, and is proven by
  validate_rejects_invalid_disabled_js_asset_proxy_assets
  (config.rs:1424). Naming that pattern here would keep an implementer from
  writing a rule that never fires.

  On sourcing: the “invalid enabled config must not be silently
  logged-and-disabled” rule is not actually in AGENTS.md or
  CONTRIBUTING.md. Its canonical statement is a HIGH-severity finding in
  docs/superpowers/specs/2026-03-11-production-readiness-report-design.md:217-233.
  If this spec relies on it as normative, that is worth making explicit, since
  it was never promoted into the contributor docs.

• 📝 Section 12.4's cache invariant is already implemented, in two
  places on Fastly. The spec asks that “tests must prove that late
  response-header handlers cannot make traced content publicly cacheable”
  (line 1355). That guarantee exists today:
  apply_response_headers_with_cache_privacy
  (crates/trusted-server-core/src/response_privacy.rs:163-172) skips operator
  response_headers entries for Cache-Control and edge-cache header names
  whenever the response is already uncacheable. On Fastly there is a second
  layer after it — apply_terminal_response_effects
  (crates/trusted-server-adapter-fastly/src/main.rs:371-392) re-runs the
  privacy guards, because EC finalize and filter effects can add Set-Cookie
  later; the regression test is
  late_filter_effects_cannot_make_an_assembled_response_public. Citing both
  would let the implementation plan reuse the mechanism instead of rebuilding
  it. Note also that apply_finalize_headers is terminal on Axum, Cloudflare,
  and Spin but not on Fastly.

• 📝 A reserved-namespace classifier already exists, at the fallback
  boundary. Section 8 requires rejecting trailing slashes, extra segments,
  repeated separators, encoded separators, and ambiguous dot segments beneath
  the reserved namespace (lines 509-514). deny_admin_diagnostic_fallback
  (crates/trusted-server-core/src/ec/admin.rs:179-196) already solves that
  shape for /_ts/admin, including a bounded percent-decode-to-fixed-point
  with MAX_PERCENT_DECODE_ROUNDS = 4. It runs first inside each adapter's
  fallback rather than at the front door, but it is the pattern to lift
  forward rather than re-derive.

• 📝 Adapter-parity caveat for section 8. Several existing /_ts/*
  routes (/_ts/api/v1/*, /_ts/set-tester, /_ts/clear-tester,
  /_ts/debug/ja4) are registered only on Fastly. Section 8 requires the trace
  namespace on all four adapters, so trace routing cannot follow that
  precedent — worth stating, since it affects the sequencing in section 17.

CI Status

All checks passing on e3f371f8c.

• browser integration tests: PASS
• integration tests: PASS
• integration tests (Fastly EC lifecycle): PASS
• prepare integration artifacts: PASS
• CodeQL: PASS
• Analyze (rust): PASS
• Analyze (javascript-typescript): PASS
• Analyze (actions): PASS
• cargo fmt: PASS
• cargo test: PASS
• cargo test (axum native): PASS
• cargo test (cross-adapter parity): PASS
• cargo test (ts CLI, native): PASS
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• vitest: PASS
• format-typescript: PASS
• format-docs: PASS
• CLAUDE.md symlink guard: PASS

[aram356]

Summary

Second review pass, against head 7a82a944a (base main). The previous pass
raised twelve findings; 14c274cba addresses all twelve, and I verified each
fix against the code at this head rather than taking the replies at face value.

Two are worth calling out as substantively verified rather than merely
annotated. The new section 9.3 allowlist table is genuinely exhaustive: diffing
it mechanically against GptDiagnosticsRequestCycle gives 30 real members, 28
allowlisted, and exactly adManager and previousCreativeId excluded, with no
listed name that does not exist on the real type; the coverage keys, counters,
metadata, binding, and durations rows all match their interfaces exactly.
And the container-nesting cap moved 8 to 10, which restores two levels of
headroom over the deepest legitimate path (gpt_diagnostics.slots[].requests[].requestedSizes[][w], level 8).

One new blocking finding, introduced by the body-handling rewrite itself: the
replacement mechanism cannot produce the 413 the same bullet mandates, cites
a precedent that uses a different API, and misreads an empty body on the one
adapter that streams it. Details inline.

1 of the inline comments below carries a one-click GitHub suggestion.
The other two are prose: one names a trigger condition, one corrects an
explanation.

Blocking

🔧 wrench

• Body-emptiness mechanism cannot return 413 and misreads streamed bodies
  — see inline at
  docs/superpowers/specs/2026-09-01-mobile-ad-render-trace-endpoint-design.md:516

Non-blocking

📝 note / ⛏ nitpick

• Blob revoke trigger is undefined — see inline at
  docs/superpowers/specs/2026-09-01-mobile-ad-render-trace-endpoint-design.md:1432
• Favicon suppression is attributed to CSP rather than the <link> element
  — see inline at
  docs/superpowers/specs/2026-09-01-mobile-ad-render-trace-endpoint-design.md:1428

Cross-cutting / body-level findings

• 📝 Prior-round findings verified as resolved. Recorded so the next
  pass does not re-litigate them: the unimplementable two-second body deadline
  and one-byte read are gone; configuration validation now specifies a
  raw-config hook modelled on validate_js_asset_proxy_config
  (crates/trusted-server-core/src/config.rs:280-299) and correctly explains
  why a schema validator never fires for a disabled integration; HEAD now
  requires explicit registration on all four adapters, citing
  dispatch_head_on_named_get_route_falls_through_to_publisher_fallback; the
  405 contract now requires the trace responder to supply Allow and the
  section 12.3 hardening itself rather than inheriting a router error; the CSP
  gained img-src data: and a class-toggle-only styling rule; the auth section
  now states first-match-wins and that the fail-closed backstop covers only
  /_ts/admin; the deprecated /__ts/page-bids alias is covered, and the TSJS
  retry it refers to is real (crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1675-1692);
  and the slot-token, cookie-cap, AuctionSlot.ext, and source-enum
  paragraphs now distinguish new design from existing code. I re-verified the
  underlying code claims at this head.

• 📝 One claim in the rewrite is accurate and load-bearing enough to
  keep. The statement that Spin buffers the body while Axum buffers only JSON
  checks out against the pinned dependency: Spin reads the full body into
  Body::Once unconditionally (edgezero-adapter-spin/src/request.rs:74-80),
  and Axum branches on content type
  (edgezero-adapter-axum/src/request.rs:21-35). That asymmetry is what drives
  the blocking finding above, so it is worth keeping the sentence even after
  the mechanism changes.

CI Status

No failing checks. Several are still running against the merge commit pushed
shortly before this review; they are recorded as pending rather than treated as
findings.

• cargo fmt: PASS
• cargo test: PASS
• cargo test (axum native): PASS
• cargo test (cross-adapter parity): PASS
• cargo test (ts CLI, native): PASS
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• vitest: PASS
• format-typescript: PASS
• format-docs: PASS
• CLAUDE.md symlink guard: PASS
• Analyze (actions): PASS
• Analyze (rust): PENDING
• Analyze (javascript-typescript): PENDING
• integration tests: PENDING
• browser integration tests: PENDING
• CodeQL: SKIPPED

[aram356]

Summary

Third review pass, against head c4d664aab (base main). Commit 155d3b09f
resolves all three findings from the previous round, and I verified each fix
against the pinned edgezero v0.0.8 source rather than against the replies.

The body-validation rewrite is the substantive one. It now matches both Body
variants explicitly, requires a clean EOF to prove a streamed body empty, and
names both traps from the last round — telling implementers not to copy
into_bytes().unwrap_or_default() and not to propagate
into_bytes_bounded errors as the response. I checked that this is actually
implementable: Body is a public, non-#[non_exhaustive] enum
(edgezero-core/src/body.rs:14-17), core already matches both variants in the
exact prescribed shape (crates/trusted-server-core/src/publisher.rs:201-211),
and async handlers are available on all four adapters, with Fastly driving them
under block_on. The favicon causality and the blob-revoke timing are both
correct now.

Two findings remain. Neither is a defect in the new text; both are places where
the spec is not yet self-contained. Since this document is the contract four
implementation PRs will be built from, closing them here is cheaper than
discovering them during implementation.

Both inline comments carry a one-click GitHub suggestion.

Blocking

🔧 wrench

• Section 13 omits every body-validation status code — see inline at
  docs/superpowers/specs/2026-09-01-mobile-ad-render-trace-endpoint-design.md:1516

Non-blocking

📌 out of scope

• Deferred-cleanup rule diverges from the console export that ships today
  — see inline at
  docs/superpowers/specs/2026-09-01-mobile-ad-render-trace-endpoint-design.md:253

Cross-cutting / body-level findings

• 📝 Prior-round findings verified as resolved, recorded so a fourth
  pass does not re-litigate them. The body-validation mechanism no longer
  depends on an API that cannot produce the mandated 413, and it no longer
  inherits the into_bytes().unwrap_or_default() behavior that would read a
  non-empty streamed body as empty — which matters, because a bodiless
  fetch() POST carries no Content-Type and Axum streams exactly that case
  (edgezero-adapter-axum/src/request.rs:24-35, pinned by its own tests at
  :97-118 and :167-181). Favicon suppression is now attributed to
  <link rel="icon"> with img-src data: only permitting the load. Blob
  cleanup is now a 1000 ms setTimeout with per-download scheduling, and the
  accompanying test lines are implementable: vi.useFakeTimers is already used
  across nine JS test files, and createObjectURL/revokeObjectURL are
  already mocked at api.test.ts:540-551.

• 📝 One concern investigated and dismissed, noted so it does not
  resurface as a finding later. The 1000 ms revoke timer raises the question of
  what happens if the document is torn down before it fires. No specified flow
  navigates during a pending download: the section 6.2 storage-failure download
  happens on the publisher page, which that section says "remains in place",
  and the section 6.3 download happens on /_ts/trace, whose own
  Copy/Share/Download and clear actions do not navigate. Line 1729's "same-tab
  navigation occurs only after a successful write" governs the earlier handoff,
  before any download exists. If a document were torn down anyway, the object
  URL is reclaimed with it. No change needed.

CI Status

All 20 checks passing on c4d664aab.

• browser integration tests: PASS
• integration tests: PASS
• integration tests (Fastly EC lifecycle): PASS
• prepare integration artifacts: PASS
• CodeQL: PASS
• Analyze (rust): PASS
• Analyze (javascript-typescript): PASS
• Analyze (actions): PASS
• cargo fmt: PASS
• cargo test: PASS
• cargo test (axum native): PASS
• cargo test (cross-adapter parity): PASS
• cargo test (ts CLI, native): PASS
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• vitest: PASS
• format-typescript: PASS
• format-docs: PASS
• CLAUDE.md symlink guard: PASS

[aram356]

The October RC exposed a trace-runtime CI compatibility issue when this PR is combined with the toolchain update in #1123. Please fix the installer in this owning PR so the RC can consume the upstream correction.

The failing RC check is trace runtime and browser acceptance. Cloudflare and Fastly boundary probes pass; Spin times out during startup, before browser acceptance runs.

[aram356]

Summary

Eighth review pass, against head ad1b8ef0b (base main). The branch is fully
current with main (the merge-base is origin/main itself), mergeable, and CI
is 24/24.

What this round verified as resolved: trace-assets.test.mjs no longer crashes
on a clean checkout, which I reproduced from a fresh npm ci with no dist/
(the comparison skips locally and runs in CI, and
trace-build-inputs.test.mjs independently regenerates the assets). The merge
resolutions in 944c3eb52 and ffb40704c are clean under --remerge-diff:
settings.rs takes main's #1326 fix wholesale, hex::encode is
output-identical to the previous {:x}, and both per-asset digests are
unchanged, so the frozen v1 bytes hold. The Spin installer from ad1b8ef0b
resolves the finding in review 5606395474: it reads the version from
.tool-versions, fails closed on an unknown pin, and its SHA-256 matches both
the release asset and upstream's published checksums-v4.1.0.txt.

I also re-proved the router invariants at the new EdgeZero rev. The pre-dispatch
hook still runs before find_route and before all middleware, for every method,
and a hook response still short-circuits. The router.rs diff from 965d482f
to 70876c92 is cosmetic, and all four adapters still populate
RequestIngress.

The headline finding is the EdgeZero dependency. It is not fixable in-branch:
the pinned rev sits on an open EdgeZero PR whose head has already changed the
API this PR uses. Details inline on Cargo.toml.

1 of the 7 inline comments carries a one-click suggestion.

Blocking

🔧 wrench

• EdgeZero pin targets an open PR whose head is already incompatible —
  see inline at Cargo.toml:64
• trusted_client_ip behaves three different ways across one feature —
  see inline at crates/trusted-server-core/src/trace/dispatch.rs:137
• Spec no longer describes asset regeneration; geo and plan drift — see
  inline at
  docs/superpowers/specs/2026-09-01-mobile-ad-render-trace-endpoint-design.md:570

Non-blocking

🤔 thinking / 🌱 seedling / 📝 note / ⛏ nitpick

• Source digest is over-broad and under-broad — see inline at
  crates/trusted-server-js/lib/trace-asset-sources.mjs:32
• No tooling behind “add a version after publication” — see
  inline at crates/trusted-server-js/build.rs:326
• Cloudflare serves path aliases Fastly rejects — see inline at
  crates/trusted-server-core/src/trace/routes.rs:207
• Digest assertion does not name the recovery command — see inline at
  crates/trusted-server-js/lib/test/trace-assets.test.mjs:21

Cross-cutting / body-level findings

• 📝 Surfaces checked this round and clean, so a ninth pass does not
  re-litigate them. The Rust and JS source digests agree: same inputs, byte-order
  sort, NUL separators, nested node_modules resolution, and both compute
  77662b63… against the committed manifest. The digest is
  host-independent, because it reads only the lockfile, which lists every
  platform optional dependency. authenticated_client_ip matches Fastly's
  resolve_client_ip on comma lists, :port, brackets, zone IDs and
  whitespace, all of which fall back to the runtime address. On Fastly it
  derives the same IP as the native path, so geo is kept. No Python files or
  invocations were added.

CI Status

All 24 checks passing on ad1b8ef0b.

[aram356]

Summary

Ninth review pass, against head 20b057537 (base main). The branch is current
with main, mergeable, and CI is 24/24.

Commit 06c86df8f resolves six of the seven findings from the previous round.
I verified each against the code, not the thread replies:

• Client IP. TraceCaptureGate now carries the reader address resolved
  before header sanitation, and trace_document_bootstrap prefers it with the
  same geo omission. The new publisher test asserts the masked address in the
  rendered document, so a regression in the override would fail it. Both docs
  pages now agree with each other and with the code.
• Digest policy. source_sha256 is enforced only on the prebuilt path, the
  Node pin is out of the digest, and css: { postcss: {} } closes PostCSS
  discovery. trace-build-inputs.test.mjs now fails if either regresses.
• Spec and plan now describe the regeneration command, the prebuilt path,
  the unpublished-replacement policy, and the geo omission behind a forwarder.
• The versioning gap has a tracked follow-up plan, and the Cloudflare alias
  divergence is documented in trace/routes.rs.

What remains blocking is the EdgeZero dependency. It is unchanged in substance,
and the author agrees it is a pre-merge blocker. It is re-raised here only
because its previous thread was resolved while the reply said it would stay
open. The other two findings are small.

1 of the 3 inline comments carries a one-click suggestion.

Blocking

🔧 wrench

• EdgeZero pin still targets open #403; restore the open-thread guard
  — see inline at Cargo.toml:64

Non-blocking

🤔 thinking / ⛏ nitpick

• Rust half of the digest pair no longer runs in CI — see inline at
  crates/trusted-server-js/build.rs:324
• Follow-up link pinned to a PR-branch commit — see inline at
  docs/guide/integrations/gpt-diagnostics.md:315

Cross-cutting / body-level findings

• 📝 Surfaces checked this round and clean, so a tenth pass does not
  re-litigate them:
  • The manifest restamp changed only source_sha256; the frozen v1 bytes and
    both per-asset digests are untouched.
  • chrono's wasmbind feature is active in the Cloudflare wasm build
    (crates/trusted-server-core/Cargo.toml:63), so the trace paths' Utc::now()
    goes through js-sys rather than the SystemTime::now() that panics on
    wasm32-unknown-unknown.
  • All 61 review threads were resolved at the time of this review.

CI Status

All 24 checks passing on 20b057537.

[aram356]

Summary

Tenth review pass, against head 9ef51f375 (base main). The branch is current
with main and has no conflicts. CI is 24/24.

All three findings from the previous round are resolved. I checked each one
against the code and the CI logs, not the thread replies:

• EdgeZero API drift. The pin d05d5526 matches the current head of
  IABTechLab/edgezero#403 (34750022) except for Cargo.lock. Every
  RequestIngress::new call site and every adapter hook registration now uses
  that API.
• Rust digest parity in CI. The new "Verify prebuilt trace source digest
  parity" step in the Axum job builds the bundles into a temporary directory
  and runs TSJS_PREBUILT_DIR=<dir> cargo check-axum --locked. It ran to
  completion on this head.
• Docs link. docs/guide/integrations/gpt-diagnostics.md:315 now points at
  blob/main.

The only remaining blocker is the EdgeZero release. Nothing in this branch has
to change until #403 ships in a tag.

Blocking

🔧 wrench

• EdgeZero pin still targets the unmerged #403: tracked in the open
  Cargo.toml thread, so this review adds no new inline comment.
  IABTechLab/edgezero#403 is still open and unmerged, and the newest tag is
  v0.0.10. Once #403 is released by tag, repin all edgezero-* dependencies
  to that tag, migrate any API changes made before release, and rerun CI.
  Please keep that thread open until then.

Cross-cutting / body-level findings

• 📝 Surfaces checked this round and clean, so the next pass does not
  re-litigate them:
  • Ingress migration. All 13 RequestIngress::new sites are test-only,
    and none dropped a header override. The Fastly fidelity change at the new
    rev touches only global_field_order. trace/cookies.rs reads only the
    octets and field_multiplicity axes, so cookie classification is
    unchanged.
  • Router hook. Hook dispatch is the same at the new rev. Each production
    router registers exactly one hook, so the new double-registration panic in
    pre_dispatch_hook_arc cannot fire.
  • view-model.ts correlation fix. This fix was not mentioned in the
    replies. Correlation links now require a matching requestNumber as well
    as runtimeSlotNumber, which is what the spec requires. Reverting the fix
    fails both new cases in test/trace/view-model.test.ts. The resulting
    v1.js regeneration is allowed while v1 is unpublished, and the CI byte
    comparison confirms the committed bytes.
  • Real-runtime capture. trace_runtime_boundary_{cloudflare,fastly,spin}
    and the browser workflow passed on this head.
  • Threads. 63 of 64 review threads are resolved. The open one is the
    EdgeZero gate above.

CI Status

• Analyze (actions): PASS
• Analyze (javascript-typescript): PASS
• Analyze (python): PASS
• Analyze (rust): PASS
• browser integration tests: PASS
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• cargo fmt: PASS
• cargo test: PASS
• cargo test (axum native): PASS
• cargo test (cross-adapter parity): PASS
• cargo test (ts CLI, native) (macos-latest): PASS
• cargo test (ts CLI, native) (ubuntu-latest): PASS
• CLAUDE.md symlink guard: PASS
• CodeQL: PASS
• docs-proposal scripts: PASS
• format-docs: PASS
• format-typescript: PASS
• integration tests: PASS
• integration tests (Fastly EC lifecycle): PASS
• prepare integration artifacts: PASS
• trace runtime and browser acceptance: PASS
• vitest: PASS

[jevansnyc]

👍

[aram356]

Summary

Eleventh review pass. The head is still 9ef51f375, the same as round 10, but main has moved by three PRs: #1222, #1413 and #1290. The branch now conflicts with main in nine files. GitHub does not run pull_request workflows on a conflicting PR, so the passing checks below predate those merges, and nothing has tested the merged tree.

To see what a textual merge would hide, I resolved the conflicts in a scratch worktree and built and tested the result. Before the semantic fixes below, that produced 12 compile errors and 3 test failures. After them, everything passed:

• 3,277 native core tests
• cargo test-axum, test-cloudflare, test-spin and test-fastly (Viceroy)
• 28 parity tests

The three inline comments are the places where a resolution can compile and still be wrong. The EdgeZero release gate from earlier rounds also still stands, and #403 has drifted again.

Blocking

🔧 wrench

• Keep #1413's host validation on whichever source is selected: see inline at crates/trusted-server-core/src/http_util.rs:190
• #1290's async EOF driver must carry trace_bootstrap: see inline at crates/trusted-server-core/src/publisher.rs:1755
• Two process-global loggers in one test binary: see inline at crates/trusted-server-core/src/trace/routes.rs:802

Cross-cutting / body-level findings

• 🔧 Conflicts with main; the merged tree has not run CI. In addition to the three inline items, the scratch merge needed these changes:

  • crates/trusted-server-integration-tests/tests/parity.rs: both sides append at the end of the file, so the conflict hunks interleave two unrelated additions. Take main's file and append this PR's trace_parity module.
  • crates/trusted-server-integration-tests/Cargo.toml: the automatic merge produces duplicate base64 and futures keys in [dev-dependencies], so Cargo cannot load the workspace. The conflicted lines need both temp-env and tokio's net, rt-multi-thread and signal features.
  • crates/trusted-server-core/src/settings.rs: main moved the test-module imports to the top of mod tests. Keep only the new trusted_forwarder tests from this side.
  • crates/trusted-server-core/src/publisher.rs:
    • Take the union of the imports.
    • Take the union of the full-body condition: diagnostics_requires_full_body and main's async_html_finish_present && GET && is_html_document_request.
    • The four AuctionDispatch literals this PR adds in tests need main's new integration_registry field.
    • main's synchronous-control test needs trace_bootstrap: None.
  • CHANGELOG.md: main's entry for #1206 to #1208 says host selection "remains Forwarded (host=), then X-Forwarded-Host, then Host" and "Forwarded-header trust policy is unchanged". This PR's entry says the opposite. Keeping both lines would put a contradiction in the unreleased section, so main's entry needs rewriting to the merged order (authenticated forwarder, then ingress or runtime authority, then Host, then URI authority, all validated).
  • docs/guide/ts-dev-proxy.md: keep this PR's trust-model paragraph. Carry over main's statements about returning 400 for a malformed host, URI-authority fallback and the empty Host, but not its forwarded-precedence claim.
  • Cargo.lock: regenerate after the manifest merge.

• 🔧 The EdgeZero dependency still targets the unmerged IABTechLab/edgezero#403, and #403 has drifted again. #403 is open with changes requested, and v0.0.10 is still the newest tag. Its head has moved from 34750022 to f6d0ed68 ("tighten pre-dispatch contract"). Compared with the pinned d05d5526, that head changes three things:

  • It renames edgezero_core::request to edgezero_core::ingress. This branch has 49 references in 10 files.
  • It makes edgezero_adapter_fastly::request::into_core_request_with_ingress pub(crate). crates/trusted-server-adapter-fastly/src/main.rs:4, :486 and :1193 depend on it. The public into_core_request(req) captures ingress during conversion. That is equivalent today, because nothing between the capture at main.rs:174 and the conversion at :486 changes the URL or Host. If the adapter needs to keep supplying a pre-mutation snapshot, raise that on #403 before it merges.
  • It rejects continuing pre-dispatch hooks that change the method or path. TracePreDispatchHook changes only headers and extensions on its continuing path, so it already complies.

  The requirement is unchanged: repin to the released tag, migrate to the released API, and rerun CI.

CI Status

These results are for 9ef51f375 and finished on 2026-10-24, before main moved. No checks have run on the merged result.

• Analyze (actions): PASS
• Analyze (javascript-typescript): PASS
• Analyze (python): PASS
• Analyze (rust): PASS
• browser integration tests: PASS
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• cargo fmt: PASS
• cargo test: PASS
• cargo test (axum native): PASS
• cargo test (cross-adapter parity): PASS
• cargo test (ts CLI, native) (macos-latest): PASS
• cargo test (ts CLI, native) (ubuntu-latest): PASS
• CLAUDE.md symlink guard: PASS
• CodeQL: PASS
• docs-proposal scripts: PASS
• format-docs: PASS
• format-typescript: PASS
• integration tests: PASS
• integration tests (Fastly EC lifecycle): PASS
• prepare integration artifacts: PASS
• trace runtime and browser acceptance: PASS
• vitest: PASS

[aram356]

Summary

Twelfth review pass, against head 74c6d3b49 (base main). The branch is current with main and mergeable, and CI passes on this head.

Every round-11 finding is resolved, and I checked each against the code rather than the replies:

• Host validation on whichever source is selected. Moving validation back onto the Host branch alone makes exactly request_info_validates_host_header_and_runtime_origin_sources fail.
• trace_bootstrap on the async EOF path. Passing None from HtmlEofDriver::new makes exactly async_eof_driver_keeps_trace_bootstrap fail.
• Test logger. #1222's shared logger is now the only log::set_logger in core.
• Merge checklist. No parity test was dropped: all 28 tests from both sides are present. The integration-test manifest has no duplicate keys. The CHANGELOG states the merged host-selection order.
• EdgeZero release. All six dependencies use the released v0.0.11 tag, which resolves to #403's merge commit 87456c708.

One new blocking item, on the Fastly conversion path.

The inline comment describes its fix in prose, because the change spans several hunks.

Blocking

🔧 wrench

• The Fastly adapter re-implements the conversion EdgeZero #403 made private: see inline at crates/trusted-server-adapter-fastly/src/main.rs:330

Cross-cutting / body-level findings

• 📝 Surfaces checked this round and clean, so the next pass does not re-litigate them:
  • Both merges, checked with git show --remerge-diff.
    • 052388d36 resolves the round-11 checklist as described.
    • 74c6d3b49 also edits #1252's creative.rs fixtures onto reserved example domains. The tests keep their meaning: the space-in-host input is still unparseable, and the suffix-boundary input still ends with the excluded name without a label boundary.
    • 74c6d3b49 also changes the TrustedClientIpConfig.shared_secret row in docs/guide/configuration.md to "store resolved". That matches main, which already resolves it from the secret store (config_payload.rs).
  • EdgeZero v0.0.11.
    • #403 changed only docs and scripts after f6d0ed680.
    • Of the other changes in the release, #417 affects only EdgeZero's own GitHub actions, which this repository does not use, and #419 renames references.
    • Cargo.lock changes only the EdgeZero sources.
    • No edgezero_core::request references remain.
    • TracePreDispatchHook already meets #403's new rule that a continuing hook must not change the method or path.
  • Tests. The native core suite passes locally: 3,299 tests.

CI Status

• Analyze (actions): PASS
• Analyze (javascript-typescript): PASS
• Analyze (python): PASS
• Analyze (rust): PASS
• browser integration tests: PASS
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• cargo fmt: PASS
• cargo test: PASS
• cargo test (axum native): PASS
• cargo test (cross-adapter parity): PASS
• cargo test (ts CLI, native) (macos-latest): PASS
• cargo test (ts CLI, native) (ubuntu-latest): PASS
• CLAUDE.md symlink guard: PASS
• CodeQL: PASS
• docs-proposal scripts: PASS
• format-docs: PASS
• format-typescript: PASS
• HTML EOF runtime (Fastly): PASS
• integration tests: PASS
• integration tests (Fastly EC lifecycle): PASS
• prepare integration artifacts: PASS
• trace runtime and browser acceptance: PASS
• vitest: PASS

[aram356]

⛏ nitpick: Say why these two fixtures are built with concat!, so nobody "simplifies" them back.

The split is needed. With the split removed, ts dev lint domains reports disallowed host publisher.example,https and disallowed host publisher.example:+443, and no // allow-domain: marker can name either host (see the 📌 below). Without a comment, though, the split reads as an accident.

The comment below covers both Rust fixtures. crates/trusted-server-js/lib/test/trace/validation.test.ts:372 ([...].join(',')) could take a one-line equivalent. I checked these bytes against rustfmt --check, the domain lint on this file, and cargo check for the core tests.

Suggested change

	concat!(
	// The two `concat!` fixtures in this list keep their exact bytes. They are split
	// because `ts dev lint domains` reads their hosts as `publisher.example,https` and
	// `publisher.example:+443`, which an `allow-domain` marker cannot name.
	concat!(

[aram356]

📌 out of scope: The reason this split exists is a gap in the #1254 domain linter, and it is worth a follow-up issue there rather than more workarounds here.

I ran ts dev lint domains on a scratch file:

• "https://publisher.example,https://publisher.example" is reported as disallowed host publisher.example,https.
• "https://publisher.example:+443" is reported as disallowed host publisher.example:+443.
• "https://publisher.example:65536" is reported as disallowed host publisher.example:65536. That one can be suppressed, which is why line 579 uses a marker.

In each case the host token keeps the trailing ,https or the invalid port, so the reserved .example TLD check (RESERVED_TLDS) no longer matches. The suppression grammar then cannot name the token:

• // allow-domain: publisher.example,https splits on the comma into publisher.example and https, and neither matches.
• // allow-domain: publisher.example:+443 is not parsed at all, because + is outside the marker's character class.

Malformed-origin fixtures on reserved hosts are a normal part of security tests. Possible fixes in the linter: test the reserved TLD against the host part before an invalid port or separator, or let the marker name the exact reported token. No change is needed in this PR.