Implement server-side ad slot templates with PBS and APS auction#680
Implement server-side ad slot templates with PBS and APS auction#680prk-Jr wants to merge 103 commits into
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Incorporate all review feedback (aram356 + jevansnyc): cache contract, consent/GDPR gating, async restructuring detail, CreativeOpportunityFormat schema, glob pattern fix, XSS escaping, win notifications, APS params, timeout config key, defineSlot fix, gpt.rs ownership, KV migration path, Phase 2 sketch - Fix Prettier formatting (format-docs CI) - Add implementation plan (12 tasks, TDD, ordered by dependency)
- Incorporate all review feedback (aram356 + jevansnyc): cache contract, consent/GDPR gating, async restructuring detail, CreativeOpportunityFormat schema, glob pattern fix, XSS escaping, win notifications, APS params, timeout config key, defineSlot fix, gpt.rs ownership, KV migration path, Phase 2 sketch - Fix Prettier formatting (format-docs CI) - Add implementation plan (12 tasks, TDD, ordered by dependency)
Replace the head-injected __ts_bids design with a server-cached bid delivery model fetched by the client via a new /ts-bids endpoint. The auction never blocks page rendering — </head> flushes immediately, body parses without waiting for bids, and the client fetches bids in parallel with content paint. Key changes: - §2 Goal: bid delivery decoupled from page rendering; FCP unchanged from no-TS baseline - §4.3 Auction Trigger: drop buffered/streaming dichotomy; single mode forces chunked encoding on all origins (WordPress, NextJS, etc.) - §4.4 Head Injection: only __ts_ad_slots and __ts_request_id injected at <head> open; bid results moved to /ts-bids endpoint - §4.6 Client Residual: __tsAdInit defines slots immediately, fetches bids via /ts-bids, applies targeting and fires refresh() after resolve - §4.7 (new) Caching Behavior: explicit cacheability table for HTML, JS, CSS, tsjs bundle, bid results; Fastly edge HTTP cache leveraged for origin HTML - §5 Request-Time Sequence: full mermaid diagram covering content + creative + burl flow with cache-hit and cache-miss branches; separate text sequences for cache-hit (~80ms FCP, ~900ms ad-visible) and cache-miss (~250ms FCP, ~1,050ms ad-visible) - §6 Performance Summary: cache-hit and cache-miss columns; FCP added as a tracked metric - §7 Implementation Scope: add bid_cache.rs, /ts-bids endpoint, force chunked encoding step - §8 Edge Cases: origin-agnostic entries; new entries for /ts-bids 404 and client-never-fetches-/ts-bids Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pivot from the /ts-bids fetch endpoint + in-process bid_cache design to
inline __ts_bids injection before </body>. The earlier design relied on
shared state that doesn't reliably survive Fastly Compute's per-request Wasm
isolate model — body injection achieves the same FCP property in a single
response with no shared-state requirement.
Key changes:
- §4.3: replace /ts-bids long-poll with bounded </body> hold tied to
A_deadline. Body content above </body> paints first; close-tag held
until auction completes or A_deadline fires (graceful __ts_bids = {}
fallback).
- §4.3: add auction-eligibility gating (consent, bot UA, prefetch hints,
HEAD method, slot match) so auctions fire on real first-page-load
impressions only.
- §4.4: replace __ts_request_id + /ts-bids machinery with two inline
<script> blocks — __ts_ad_slots at <head> open, __ts_bids before
</body> via lol_html el.on_end_tag().
- §4.5: move both nurl and burl to client-side firing from
slotRenderEnded after hb_adid match. Server-side firing rejected to
avoid billing inflation on bids that never render.
- §4.6: replace fetch+Promise pattern with synchronous __ts_bids read.
Add lazy slim-Prebid loader (post-window.load) for scroll/refresh
auctions and Phase B identity warm-up. Add ts_initial=1 slot-ownership
sentinel.
- §4.7: switch Cache-Control from private, no-store to private,
max-age=0 to preserve browser BFCache eligibility while still
preventing intermediate-cache leaks.
- §4.8 (new): document the EC/KV identity model as load-bearing auction
input — Phase A retrieval at request time, Phase B post-render
enrichment via slim-Prebid userID modules. Add bare-EC first-impression
caveat and auction_eid_count metric. Note federated-consortium
passphrase property and clickstream-compounding speed win.
- §5: update mermaid + cache-hit/miss timelines for bounded body hold;
ad-visible converges to ~870ms (hit) / ~1,020ms (miss).
- §6: drop /ts-bids RTT row; add DCL row; add clickstream-compounding,
TS-overhead, identity-coverage, and confidence-interval framing.
- §7: drop bid_cache.rs and /ts-bids endpoint from scope; add
auction-eligibility gating and slim-Prebid bundle build target. Add
explicit "Deleted" subsection.
- §8: drop /ts-bids edge cases; add SPA/pushState, bare-EC, bot/prefetch,
HEAD, BFCache restoration cases.
- §9.6: server-side GAM downgraded from "Phase 2 commitment" to
aspirational and contingent on Google agreement. §9.8 (slim-Prebid
bundle composition), §9.9 (Privacy Sandbox), §9.10 (per-bidder consent)
added as follow-ups.
Implementation plan at docs/superpowers/plans/2026-04-30-server-side-ad-templates.md
is now stale relative to this spec; needs regenerating before code lands.
…ities.toml Adds the creative_opportunities field to Settings struct to deserialize configuration for the server-side ad auction feature. Includes build.rs stubs for types required during build-time configuration validation. Creates creative-opportunities.toml with example slot configuration and updates trusted-server.toml with the [creative_opportunities] section defining GAM network ID, auction timeout, and price granularity settings. Tests pass with proper TOML parsing of the creative_opportunities section.
…ared auction state
- Add `ad_slots_script: Option<String>` and `ad_bids_state: Arc<RwLock<Option<String>>>` fields to `HtmlProcessorConfig`
- Update `from_settings` to initialize both new fields with safe defaults
- Prepend `ad_slots_script` inside the existing `<head>` handler before integration inserts
- Add `element!("body", ...)` handler that uses `end_tag_handlers()` to inject `__ts_bids` before `</body>`; falls back to empty `{}` when auction state is `None`
- Add `IntegrationRegistry::empty_for_tests()` test helper
- Add three new tests covering all injection paths
…gibility gates; max-age=0 - Make handle_publisher_request async; add orchestrator and slots_file params - Dispatch origin request with send_async before running auction in parallel - Gate auction on GET, no prefetch, no bot, matched slots, TCF purpose-1 consent - Run server-side auction and write bucketed bids to ad_bids_state Arc<RwLock> - Compute ad_slots_script after response headers; set Cache-Control: private, max-age=0 - Fix Stream arm to thread actual ad_slots_script and ad_bids_state through - Add build_auction_request, build_bid_map, build_bids_script, build_ad_slots_script helpers - Update route_tests.rs to pass empty slots_file to route_request
…m slotRenderEnded
- build_bid_map now returns serde_json::Map with full bid objects (hb_pb,
hb_bidder, hb_adid, nurl, burl) instead of a plain CPM string map
- build_bids_script / build_ad_slots_script now emit full <script> tags
using JSON.parse("…") for safe inline embedding; add html_escape_for_script helper
- build_ad_slots_script uses correct property names (gam_unit_path, div_id,
formats, targeting) matching the client-side TSJS bundle expectations
- Replace map_or(false, …) with is_some_and(…) on lines 546, 549, 567
- Add # Panics doc sections to handle_publisher_request and create_html_processor
…nities.toml at startup
… from slotRenderEnded; slim-Prebid lazy loader
- Enable APS and adserver_mock in auction config; set providers and mediator - Increase auction_timeout_ms from 500ms to 3000ms — 500ms was too tight for HTTPS round-trips to mocktioneer, leaving the mediator zero budget - Fix mediation request: send numeric price instead of opaque encoded_price; mocktioneer requires a decoded price field and does not support encoded_price - Expand creative-opportunities slot page_patterns to include /news/**
Define SlotRenderEndedEvent, SlotRenderEvent, and TestWindow types to eliminate all @typescript-eslint/no-explicit-any violations in gpt/index.ts and gpt/index.test.ts. Extend GptWindow with __tsjs_slim_prebid_url so installSlimPrebidLoader avoids the any cast.
prk-Jr
changed the title
Set gam_network_id to 88059007 (autoblog production network). Update atf_sidebar_ad slot to /88059007/autoblog/news with div_id ad-atf_sidebar-0-_r_2_ (desktop ATF sidebar, 300x250); restrict page_patterns to article paths only (/20**, /news/**) since that div does not exist on the homepage. Add homepage_header_ad slot targeting /88059007/autoblog/homepage with ad-header-0-_R_jpalubtak5lb_ for 970x90/728x90/970x250 leaderboard formats. Reduce auction_timeout_ms from 3000 to 500 to cap TTFB at the spec-recommended ceiling.
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Automated review: I reviewed PR #680 at def951a against main. CI is green, but I found blocking privacy/security and correctness issues in the new server-side ad-template paths. In particular, the publisher/page-bids auction paths can forward persistent identifiers after EC consent is denied, the default config now enables automatic outbound ad calls to real endpoints, and the GPT/Prebid refresh paths can duplicate requests or reuse stale targeting. Details are inline.
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Summary
Reviewed the current branch PR #680 at 80fe39220 against main. I found several remaining issues that should be addressed in this PR because they affect the new server-side ad-template auction behavior: consent gating, the auction kill switch, PBS URL compatibility, SPA refresh flow, timeout budgets, and stale GPT targeting.
Per discussion, the raw browser-cookie forwarding concern is not included in this PR review and is tracked separately in #763.
Findings submitted inline: 0 P0, 4 P1, 1 P2, 1 P3. One additional P2 is included below because the affected provider timeout lines are not part of the final PR diff.
P2 — Provider payload timeouts ignore the effective auction budget
crates/trusted-server-core/src/integrations/prebid.rs:1171 and crates/trusted-server-core/src/integrations/aps.rs:367 still use provider config timeouts in the payload (tmax / APS timeout), while the Fastly backend timeout is capped to context.timeout_ms. When creative_opportunities.auction_timeout_ms is lower than the provider timeout, providers are told they have more time than the edge will actually wait, which can turn useful partial bids into edge timeouts. Please use the effective context.timeout_ms in provider payloads and add tests where provider config is 1000ms but auction context is 500ms.
aram356
left a comment
aram356
left a comment
There was a problem hiding this comment.
Summary
Server-side ad slot templates with PBS/APS auction — a large, well-tested feature (44 files, +12,746/−395). The XSS escaping, single-injection guard, consent gating, and price-bucket math are solid. Findings cluster around win/billing beacon correctness (the headline issue), a runtime slot-id validation gap, and JS test hygiene. Inline comments carry most findings; cross-cutting items are below.
Blocking
🔧 wrench
- Win/billing beacons over-fire / double-fire —
gpt_bootstrap.js:95andgpt/index.ts:440(inline). validate_slot_iddead at runtime — env-injected slot ids unvalidated,creative_opportunities.rs:270(inline).- JS test file leaks global
addEventListener; suites duplicated across two dirs —gpt/index.test.ts:10(inline). installSpaAuctionHookhas zero coverage —gpt/index.ts:475(inline).
❓ question
- Prebid stored-request fan-out behavioral change —
prebid.rs:974(inline). - page-bids
pathquery param not normalized before glob match —publisher.rs:1584(inline). - Cache-Control
max-age=0vs claimedno-store—publisher.rs:1214(inline).
Non-blocking (not pinnable to a single diff line)
🤔 thinking
- collect vs parallel auction paths may diverge —
auction/orchestrator.rscollect path callsprovider.parse_response(...)whilerun_providers_parallelcallsparse_response_with_context(...). If any provider's context-aware parse does extra work (consent/settings-driven), the async and parallel paths produce different bids for the same upstream response. Confirm the plainparse_responseis an equivalent superset, or thread context through the collect loop too.
🌱 seedling
- No per-iteration deadline check in the collect select-loop —
auction/orchestrator.rscollect_dispatched_auctionrelies entirely on each backend's dispatch-time timeout cap, whereasrun_providers_parallelre-checkselapsed >= deadlineafter eachselect(). The invariant holds today (dispatch caps each provider at the remaining budget), but the two paths diverge silently; mirror the guard for defense-in-depth.
♻️ refactor
adserver_mock.rs:101carries request-scopedbid_indexviaMutexon a sharedArcprovider — same pattern flagged inline onaps.rs:289; migrate toparse_response_with_context.
⛏ nitpick
platform_response_to_fastlyis infallible but returnsResult—auction/orchestrator.rs:497; every call site handles anErrthat can never fire. Returnfastly::Responsedirectly or document the forward-compat intent.- Redundant constructors —
PriceGranularity::dense()(returnsSelf::Dense, and the enum alreadyDefaults toDense) andMediaType::banner()add public API surface over the variant literals for no benefit.
CI Status
- cargo fmt: PASS
- cargo clippy: PASS
- cargo test: PASS
- vitest (JS): PASS
- CodeQL / integration / browser-integration: PASS
- Fail closed on consent: add consent_allows_server_side_auction() helper requiring effective TCF/GPP Purpose 1 for GDPR and unknown jurisdictions (or any request carrying an EU TCF signal); used by both the publisher navigation auction and /__ts/page-bids - Pass the adapter's geo-aware EcContext into handle_page_bids so the jurisdiction decision sees real geo instead of always-unknown - Make [auction].enabled a real kill switch for the automatic publisher navigation ad stack and the /__ts/page-bids auction - Normalize Prebid server_url: use it as-is when it already ends with /openrtb2/auction, otherwise append the path (backward compatible) - Advertise the effective auction budget in provider payloads: PBS tmax and APS timeout now use the orchestrator-capped context.timeout_ms instead of raw provider config - Add a one-shot adInitRefreshInProgress bypass so slim-Prebid's refresh wrapper passes adInit()'s internal refresh straight to GPT instead of clearing server-side targeting with a duplicate client-side auction - Sweep stale TS targeting (hb_*, ts_initial, route keys) from all previously TS-touched GPT slots before applying a new SPA route - Map the GPT slot element ID (container div) in the inline bootstrap's divToSlotId so container-backed slots fire nurl/burl beacons
- Dedupe win/billing beacons: fire each bid's nurl/burl at most once, keyed by slot + bid identity in shared tsjs state so the inline bootstrap and bundle listeners can never double-fire; unify the ourBidWon check (hb_adid confirmation with hb_bidder fallback for APS bids) across both implementations - Wire validate_slot_id into Settings::prepare_runtime so every load path (including env-injected slots on runtime-config adapters) rejects invalid slot IDs; build.rs settings stub gains a no-op - Normalize the client-controlled page-bids path parameter: strip query/fragment and force a leading slash before glob matching - Document the deliberate Cache-Control private, max-age=0 choice (BFCache eligibility per design spec section 4.7, not no-store) - Align orchestrator collect path with the parallel path: use parse_response_with_context for providers and the mediator, and add a defense-in-depth deadline check to the collect select-loop - Migrate adserver_mock off request-scoped Mutex state: the SSP bid index is rebuilt in parse_response_with_context from the context's provider responses; document why APS's slot_id_map cannot follow yet - Make platform_response_to_fastly infallible; drop the dead error arms - Remove redundant PriceGranularity::dense and MediaType::banner constructors in favor of Default-based serde field defaults - Clarify that the Prebid stored-request fallback cannot fire for the client /auction path (every ad unit carries a trustedServer entry) - Consolidate GPT JS suites under test/integrations/gpt/, replace the leaked module-scope addEventListener patch with a restored wrapper, and add installSpaAuctionHook coverage (pushState/replaceState/ popstate, stale-response guard, non-OK response, idempotence)
The edge injects adSlots and bids only when the server-side ad stack
runs for the request. When it is gated off (kill switch, consent
fail-closed, bots, prefetch), page code reading window.tsjs.bids or
window.tsjs.adSlots previously threw on undefined and could blank
SPA rendering. Core init now defaults them to {} / [] without
clobbering edge-injected values that arrive before the bundle.
Also flip the committed [auction] enabled default to true: with all
provider integrations disabled and no slot templates in the checked-in
config, the flag alone dispatches nothing, and the kill switch now
actually gating the ad stack means a false default breaks every
local/dev setup that enables a provider without noticing the flag.
|
All findings from the two latest reviews are addressed in d3d43bc, 0f4dd86, and aa43944. Inline threads have individual replies; the items below had no inline thread. @ChristianPavilonis — P2 provider payload timeouts (review body): Fixed in d3d43bc. PBS @aram356 — blocking:
@aram356 — questions:
@aram356 — non-blocking:
Deliberately deferred: gating aa43944 additionally defaults CI: cargo fmt / clippy |
…tes-impl # Conflicts: # crates/trusted-server-adapter-fastly/src/main.rs # crates/trusted-server-core/build.rs # crates/trusted-server-core/src/publisher.rs # crates/trusted-server-core/src/settings.rs
Operator [response_headers] could reintroduce Surrogate-Control / Fastly-Surrogate-Control on responses the publisher path had marked private, making per-user HTML eligible for shared surrogate caching again. finalize_response now skips Cache-Control and both surrogate headers whenever the response already carries a private directive. GET /__ts/page-bids dispatched real PBS/APS auctions with no same-origin check, so any third-party page could trigger partner calls from a visitor's browser. The handler now requires Sec-Fetch-Site: same-origin/same-site, or the non-simple X-TSJS-Page-Bids header when fetch metadata is absent (legacy clients), and returns 403 otherwise. The tsjs SPA hook sends the header on every page-bids fetch.
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Automated review:
Review Summary
Reviewed PR #680 at 9613aff against origin/main, including current CI/review context and focused passes over Rust auction/publisher paths, GPT/Prebid JS, config/build validation, and caching/security behavior. CI is currently green, and many prior review findings have been fixed. I still found blocking correctness/compatibility issues in the server-side ad-template flow, especially around win/billing beacons and cache behavior.
Findings
P0 / Blockers
None.
P1 / High
-
Win and billing beacons can fire for non-Trusted-Server winners —
crates/js/lib/src/integrations/gpt/index.ts:463- Issue: The
slotRenderEndedhandler treats retained GPT slot targeting as proof that the TS bid won. PBS bids comparehb_adidagainstslot.getTargeting('hb_adid'), and APS bids only require!!bid.hb_bidder. Slot targeting is request state, not proof of the winning GAM line item; direct/sponsorship/house line items can render non-empty while TS targeting remains on the slot. - Why it matters:
nurl/burlare win/billing notifications. False positives can over-report wins and trigger billing for impressions won by other GAM demand. APS is especially exposed because the fallback has no event/creative identity check. - Suggested fix: Fire beacons only from a positive TS creative signal, such as a TS/Prebid creative callback carrying the expected ad/cache id, or a configured line-item/creative-id allowlist checked against
slotRenderEndedevent fields. Add a test where a non-empty non-TS line item renders while TS targeting remains on the slot.
- Issue: The
-
Ad-template cache policy is applied to every HTML response, even when the stack is disabled/no-op —
crates/trusted-server-core/src/publisher.rs:1219- Issue: All proxied
text/htmlresponses are rewritten toCache-Control: private, max-age=0and have surrogate headers stripped whenever this path handles HTML, regardless of whether any creative-opportunity slot matched or whether the checked-in/default config has zero slots. - Why it matters: Deploying with the current default/empty slot config can silently remove public/shared cacheability from all publisher HTML even though no per-user
tsjs.adSlots/tsjs.bidsdata was injected. That is a severe compatibility/performance regression and undermines the empty-slot kill-switch behavior. - Suggested fix: Only force private/no-surrogate for responses that actually need per-user protection (for example when matched slots/ad scripts/bids are injected, or another per-user response mutation requires it). Add tests for zero-slot and non-matching-slot configs preserving origin cache headers unless per-user data is injected.
- Issue: All proxied
P2 / Medium
-
/__ts/page-bidsaccepts same-site cross-origin callers —crates/trusted-server-core/src/publisher.rs:1580- Issue:
Sec-Fetch-Site: same-siteincludes sibling origins under the same registrable domain. This endpoint is side-effecting and can dispatch real PBS/APS auctions while forwarding request-derived signals to partners. - Why it matters: An untrusted same-site sibling origin cannot read the JSON without CORS, but it can still trigger partner calls and burn SSP quota from the visitor's browser context.
- Suggested fix: Require
Sec-Fetch-Site: same-originfor Fetch-Metadata-capable browsers. Keep the legacy fallback only whenSec-Fetch-Siteis absent and the non-simpleX-TSJS-Page-Bidsheader is present.
- Issue:
-
Repeated
requestBids()calls can drop inline server-side bidder params —crates/js/lib/src/integrations/prebid/index.ts:496- Issue: The shim mutates
unit.bidsin-place to keep onlytrustedServerand client-side bidders. On the nextrequestBids()call with the same Prebid ad unit object, the original server-side bidder entries are gone, sobidderParamsis rebuilt as{}and overwrites the existing trustedServer bidder params. - Why it matters: Publishers that supplied inline PBS params on the initial ad unit can lose them on refresh/re-auction and fall back to empty params/stored-request behavior instead.
- Suggested fix: Preserve original server-side bids in a side table/non-destructive clone, or retain existing
trustedServer.params.bidderParamswhen no new server-side bids are found. Add a test that callsrequestBids()twice with the same ad unit.
- Issue: The shim mutates
-
SPA auctions can apply before the new route's ad containers exist —
crates/js/lib/src/integrations/gpt/index.ts:548- Issue: After the page-bids fetch resolves, the SPA hook immediately writes
ts.adSlots/ts.bidsand callsts.adInit().adInit()looks up each slot element once and silently skips missing elements. Many SPA routers update history before the new route DOM has committed. - Why it matters: A fast edge response can run before the ad container exists, dropping server-side bids for that route with no retry and no beacon path.
- Suggested fix: Defer route application until the next render tick and/or retry missing slots for a bounded window (for example
requestAnimationFrameplusMutationObserver/timeout). Add a test where the page-bids response resolves before the slot element is inserted.
- Issue: After the page-bids fetch resolves, the SPA hook immediately writes
-
Build-time creative-opportunity validation only checks slot IDs —
crates/trusted-server-core/build.rs:131- Issue: The build script deserializes creative-opportunity slots as raw JSON and validates only
id, while the runtime schema requires fields likepage_patternsandformatsand denies unknown fields. - Why it matters: Bad slot config can pass
cargo buildand only fail later when the embedded settings are loaded at runtime/deploy. - Suggested fix: Validate
slot_rawagainst a build-time schema that mirrors the runtime required fields/types, or reuse the real runtime deserializer in build context.
- Issue: The build script deserializes creative-opportunity slots as raw JSON and validates only
P3 / Low
None.
CI / Existing Reviews
Current GitHub checks are passing (cargo fmt/test, clippy/Analyze, vitest, browser and integration tests, CodeQL, docs/TS formatting). Existing review threads show multiple prior blocking findings have been addressed; the findings above are based on current head behavior. Inline comments were attempted first, but GitHub rejected the batch as line-unresolvable, so findings are consolidated in this review body.
Address three review findings in the server-side ad-template flow:
- page-bids gate now requires Sec-Fetch-Site: same-origin and no longer
accepts same-site, which would admit sibling origins under the same
registrable domain that are not trusted to spend SSP quota. The
absent-metadata legacy fallback (X-TSJS-Page-Bids header) is unchanged.
- The Prebid requestBids shim no longer drops inline server-side bidder
params on a second call with the same ad unit. The first call prunes
server-side bidders from unit.bids, so a later call rebuilt bidderParams
as {}; it now retains the params captured on the first call when no new
server-side bidders are present.
- The SPA navigation hook defers applying bids until the new route's ad
containers exist (MutationObserver, bounded by a 2s timeout) so a fast
edge response cannot beat the DOM and silently drop server-side bids.
P1.2: handle_publisher_request forced Cache-Control: private and stripped surrogate headers on every text/html response, so a zero-slot or non-matching navigation lost shared cacheability even though no per-user ad data was injected. Gate that rewrite on should_run_ad_stack instead. First-visit identity responses are still protected: finalize_response now downgrades any Set-Cookie response to private and strips surrogate headers, since ec_finalize_response sets the EC cookie without a cache directive of its own. Returning-visitor, cookieless, non-ad HTML keeps its origin cache headers. P1.1: the slotRenderEnded win/billing beacon fired for any non-empty render whenever an APS bid existed for the slot (the !!hb_bidder fallback), over-reporting wins and billing for impressions won by other GAM demand. Require an hb_adid match instead, in both the bundle listener and the inline bootstrap. Slot targeting is still request state rather than proof of the winning line item, but this removes the unconditional false positive.
|
Thanks for the pass — addressed the five behavioral findings across two commits (065cea6, 5b225ae). Summary of how each was resolved: P1.1 — win/billing beacons could fire for non-TS winners (5b225ae) P1.2 — cache policy applied to all HTML even when the stack is no-op (5b225ae)
P2.1 — page-bids accepted same-site cross-origin callers (065cea6) P2.2 — repeated requestBids() dropped inline server-side bidder params (065cea6) P2.3 — SPA auctions could apply before the route's containers exist (065cea6) P2.4 — build-time validation only checks slot IDs CI green locally: cargo fmt/clippy, full workspace tests (1391 core + 45 adapter), and vitest (353). |
Merging main brought back the formats.rs make_bid helper, whose Bid literal predated the cache_id/cache_host/cache_path/ad_id fields this branch added to auction::types::Bid. Set them to None so the lib test target compiles. Pure test-helper fix; no change to /auction behavior.
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Automated review: Reviewed PR #680 at 0283b13 against main. CI is green, but I found blocking correctness issues in the server-side ad-template runtime: win/billing beacons can still fire without proof that the TS bid actually won, and SPA navigation can drop later-mounted slots on multi-slot routes. I also found a medium compatibility issue in the new content-type gates.
Summary of findings:
- P1: Win/billing beacons still use GPT slot targeting as winner proof (bundle and inline bootstrap).
- P1: SPA page-bids DOM wait resolves when any slot appears, so later-rendered slots are skipped permanently.
- P2: Case-sensitive Content-Type checks can bypass processing after dispatching auctions. The inline diff comment did not attach cleanly; the affected checks are
crates/trusted-server-core/src/publisher.rsaroundorigin_content_type.contains("text/html"),params.content_type.contains("text/html"), andis_processable_content_type. MIME types are case-insensitive, so normalize the header value (or parse MIME types case-insensitively) and add coverage forText/HTML; charset=utf-8.
CI / existing reviews: all current GitHub checks pass at this head. I reviewed the prior feedback and focused on issues still present after the latest fixes.
5 participants
Summary
[creative_opportunities]intrusted-server.toml. Matching slots are selected from the incoming document URL at the edge, andwindow.tsjs.adSlotsis injected at<head>open so initial GPT setup does not need a separate slot-discovery request.window.tsjs.bidsbefore</body>.window.tsjs.adInitGPT runtime path. It readswindow.tsjs.adSlotsandwindow.tsjs.bidssynchronously, defines or reuses GPT slots, applies slot-level andhb_*targeting, setsts_initial=1, refreshes the initial slots, and firesnurl/burlonly afterslotRenderEndedconfirms the TS bid won viahb_adid.[integrations.prebid].suppress_nurl_bidders, while retaining the deployment-wide[integrations.prebid].suppress_nurlcompatibility switch.window.tsjs.adSlots/ GPT metadata instead of placeholder refresh sizes.ts-eidscookie written by TSJS, gates EIDs by consent, and forwards them as OpenRTBuser.ext.eidsto PBS.ClientInfointo the server-side PBS request so bidders see the browser client context rather than the Fastly edge context.GET /__ts/page-bidshook, which updateswindow.tsjs.adSlots/window.tsjs.bidsand re-runswindow.tsjs.adInit()afterpushState,replaceState, orpopstatenavigations.What the server-side auction sends to PBS
user.idts-eccookie or generated EC identityuser.consent/user.ext.consentuser.ext.eidsts-eidscookie plus EC identity resolutionuser.ext.ec_freshdevice.uaUser-Agentheaderdevice.ipClientInfo.client_ipdevice.geodevice.dnttrueif setDNT: 1headerdevice.languageAccept-Languageheadersite.domain/site.pagesite.refRefererheaderregs.gdpr/regs.us_privacy/regs.gppimp.*[creative_opportunities]slot templates and Prebid configtmax[creative_opportunities].auction_timeout_ms, falling back to[auction].timeout_msChanges
trusted-server.toml[creative_opportunities]slot templates and documents Prebid nurl suppression knobs..env.example/ docsSUPPRESS_NURL_BIDDERS.crates/trusted-server-core/src/creative_opportunities.rscrates/trusted-server-core/src/settings.rs/build.rstrusted-server.toml.crates/trusted-server-core/src/publisher.rswindow.tsjs.adSlotsandwindow.tsjs.bids, applies cache-control safeguards, handles page-bids responses, and forwards client context.crates/trusted-server-core/src/html_processor.rstsjs.bidsinjection.crates/trusted-server-core/src/auction/*crates/trusted-server-core/src/integrations/prebid.rsnurl/burlpropagation, and per-bidder suppression.crates/trusted-server-core/src/integrations/aps.rscrates/trusted-server-core/src/integrations/adserver_mock.rscrates/trusted-server-core/src/integrations/gpt.rs/gpt_bootstrap.jswindow.tsjs.adInitbootstrap path.crates/js/lib/src/integrations/gpt/index.tswindow.tsjs.adSlots,window.tsjs.bids, render-confirmed beacon firing, SPA updates, slim-Prebid loading, and Prebid creative rendering bridge.crates/js/lib/src/integrations/prebid/index.tscrates/trusted-server-adapter-fastly/src/main.rsCloses
Closes #677
Closes #697
Closes #698
Closes #699
Closes #700
Closes #702
Test plan
Automated
cargo test --workspacecargo clippy --workspace --all-targets --all-features -- -D warningscargo fmt --all -- --checkgit diff --checkcd crates/js/lib && node build-all.mjscd crates/js/lib && npm run lintcd crates/js/lib && npm run formatcd docs && npm run formatcd crates/js/lib && npx vitest runcurrently fails before test discovery in this workspace withERR_REQUIRE_ESMfromhtml-encoding-snifferrequiring@exodus/bytes/encoding-lite.js; no local JS test assertions execute under that failure mode.Manual end-to-end (browser DevTools console)
The steps below build on each other. Use a URL whose path matches one of the configured
[[creative_opportunities.slot]]page_patterns.Step 1 - Verify slot config is injected at
<head>openExpected: an array of slot objects. Each entry has
id,gam_unit_path,div_id,formats, andtargeting. Note thediv_idvalue from one matching slot for step 3.Step 2 - Verify server-side auction results are injected before
</body>Expected: an object keyed by slot ID. Winning slots include
hb_bidder,hb_pb, and, for Prebid cache-backed bids,hb_adid/ cache fields.Step 3 - Verify
window.tsjs.adInitwired GPT targetingReplace
SLOT_DIV_IDwith thediv_idfrom step 1.Expected: the slot has the configured GAM unit path and targeting includes
hb_pb,hb_bidder, any slot-level keys such aspos/zone, andts_initial: ["1"].Step 4 - Verify slot matching is page-pattern-aware
Navigate to a different configured path, for example
/when homepage slots are configured, and repeat step 2.Expected:
window.tsjs.bidsis keyed by the slots matching that page, not by slots from the previous page type.Step 5 - Confirm no duplicate bids injection
View page source and search for
.bids=JSON.parse.Expected: exactly one
window.tsjs.bidsassignment before</body>on pages where an auction ran.Pending (GAM line items required)
Creative delivery requires standard GAM line items targeting
hb_pb,hb_bidder, and related Prebid keys. That setup is outside this PR.Checklist
unwrap()in production codelogmacros instead ofprintln!