Render SSAT winning creative inline without PBS Cache round trip#899
Render SSAT winning creative inline without PBS Cache round trip#899prk-Jr wants to merge 10 commits into
Conversation
- Drop render_adm param: always include adm when creative present
- Gate GAM-bypass on per-bid debug_bid instead of a global window.tsjs flag
(removes SPA-staleness edge case, TsjsApi change, and whole flag-emit task)
- Correct fallback scope: cache fallback only when adm absent; render failure
after adm is supplied is not detectable
- Qualify sandbox claim: TS guarantees script-context escaping; bridge frame
isolation depends on the Prebid Universal Creative
- Reconcile with existing ad_init.test.ts coverage (rename, no duplicates)
- Fix test assertion messages to expect("should ..."); list exact clippy gates
- Task 3: assert observable DOM (GAM iframe src) instead of spying on the module-private injectAdmIntoSlot - Use existing make_bid helper + set .creative (make_test_bid_with_creative does not exist on this branch) - Hostile-adm regression test covers both U+2028 and U+2029 - Add 'cd docs && npm run format' to verification
build_bid_map now always inserts the winning creative as adm so the pbRender bridge can render it locally (no PBS Cache round trip); the verbose debug_bid blob and the GAM-bypass gate stay behind inject_adm_for_testing. Rename the param include_adm -> include_debug_bid and thread it through write_bids_to_state. Reconcile the by-default test to the new behavior, drop the now-redundant debug-only-adm test, and pin script-context escaping for a hostile adm (</script> + U+2028/U+2029).
The direct GAM-replace path (injectAdmIntoSlot) now fires only when the bid carries debug_bid, which is present only under inject_adm_for_testing. In production the always-present adm is rendered by the pbRender bridge and GAM stays in the loop. Add observable-DOM tests (GAM iframe src unchanged without debug_bid, rewritten with it), strengthen the bridge test to prove inline adm is preferred even when cache coords are present, and rename debug-adm terminology to inline adm.
aram356
left a comment
aram356
left a comment
There was a problem hiding this comment.
Review summary
The inline adm path is a useful latency improvement, and the per-bid debug gate is a good way to avoid stale SPA-global state. I am requesting changes for two production contract gaps before this ships:
- The new SSAT path bypasses the existing creative sanitization, URL-rewrite, and size-limit boundary.
- The advertised PBS Cache fallback forwards a cached bid JSON document as creative markup instead of extracting its
adm.
Additional non-blocking finding
♻️ Update the public contracts to match the new semantics. Several comments outside this diff still say adm is present only under inject_adm_for_testing: Settings::inject_adm_for_testing, AuctionBidData.adm, and the GPT integration comments at lines 279–285 and 825–835. Please update them to distinguish production inline adm from the testing-only direct GAM replacement gated by debug_bid.
Verification
- Reviewed all five changed files against base
0e5dbb211da5e61598681df3abfda239484afa5f. - Current GitHub CI is green (19 reported checks).
- Focused GPT Vitest suite: 26/26 passing.
- Focused Rust regressions: 2/2 passing.
git diff --check: clean.- Local
cargo test-fastlycompiled, but Viceroy could not access the macOS certificate keychain (No keychain is available); the corresponding GitHub Fastly job passes, so this is a local environment limitation rather than a PR failure.
| and the round trip disappears. | ||
|
|
||
| **Fallback scope (corrected):** the bridge posts the markup to the PUC and | ||
| returns; it receives **no render-success signal**. So the PBS Cache fallback |
There was a problem hiding this comment.
🤔 Non-blocking: pin and test the actual PUC protocol.
PUC 1.18.0 emits Prebid Event messages for adRenderSucceeded and adRenderFailed, so the protocol does expose a signal even though this bridge does not currently consume it. Its dynamic-renderer path also requires rendererVersion >= 3; the response here supplies renderer without a version, so that renderer is ignored by this PUC version. See messaging.js, renderingManager.js, and dynamicRenderer.js.
Please confirm and pin the PUC version/template used by GAM, add a browser contract test against that real bundle, and either send the renderer protocol it expects or remove the unused renderer field. The render-failure and beacon-timing claims in this design should then be updated accordingly.
There was a problem hiding this comment.
Deferring this to a follow-up rather than guessing. Pinning the rendererVersion/template and adding a browser contract test needs the actual PUC bundle GAM serves in production, which I can't verify from here — and sending an unconfirmed rendererVersion or dropping the renderer field could break rendering on whatever version is live. The design spec already documents that the bridge consumes no adRenderSucceeded/adRenderFailed signal and that fallback fires only on absent/empty inline adm. Happy to open a tracking issue to confirm the GAM-served PUC version and then reconcile the renderer field + render-failure claims — let me know and I'll file it.
…code cached bids The inline-adm path fed raw bid.creative into window.tsjs.bids, bypassing the creative-processing boundary the /auction path applies (sanitize_creative_html then rewrite_creative_html, which also enforces the 1 MiB creative cap). Run the same boundary in build_bid_map before inserting adm; omit adm when the creative is rejected so the bridge falls back to the PBS Cache coordinates. Thread &Settings through build_bid_map and write_bids_to_state for rewrite_creative_html. The pbRender bridge's PBS Cache fallback forwarded the raw GET body as the ad, but PBS Cache returns a JSON bid object and the Prebid Universal Creative renders bidObject.adm. Add extractCachedAdm to parse the cached bid and extract its adm, keep raw-markup compatibility, and decline to render when no adm is present. Add hostile and oversized creative coverage in Rust, and realistic returnCreative=false, malformed, and raw-markup cache-response coverage in JS. Update the inject_adm_for_testing, AuctionBidData, GPT bridge, and design-spec docs to distinguish production inline adm from the testing-only debug_bid path.
2 participants
Summary
admis now always included in
window.tsjs.bids, so the existing render bridgeserves it locally when GAM's Prebid line item fires — eliminating the
render-time PBS Cache round trip and matching how client-side
/auctionrenders from the copy the browser already holds.
hb_pb, andhb_cache_host/hb_cache_pathremain as the fallback for an absentadm.No GAM bypass in production.
debug_bidblob stays behindinject_adm_for_testing. TheGAM-bypass path (
injectAdmIntoSlot) is now gated on the per-biddebug_bidfield instead of
bid.adm— no new global flag, noTsjsApichange, correctacross SPA auction responses.
Changes
crates/trusted-server-core/src/publisher.rsbuild_bid_mapalways insertsadmwhenbid.creativeisSome; paraminclude_admrenamed toinclude_debug_bid(now gates only thedebug_bidblob);write_bids_to_stateparam renamed to match. Added a hostile-admregression test proving</script>/U+2028/U+2029cannot break out of the emitted<script>.crates/trusted-server-js/lib/src/integrations/gpt/index.tsbid.adm && bid.debug_bid; render-bridge log message updated to "from inline adm".crates/trusted-server-js/lib/test/integrations/gpt/ad_init.test.tsdebug_bidis present); renamed "debug adm" terminology to "inline/local adm"; bridge fixtures now carry bothhb_cache_*coords and inlineadm, proving the bridge prefers localadm.docs/superpowers/specs/2026-07-13-ssat-render-inline-creative-design.mddocs/superpowers/plans/2026-07-13-ssat-render-inline-creative.mdCloses
Closes #898
Test plan
cargo test-fastly && cargo test-axum(alsocargo test-cloudflare && cargo test-spin— all green)cargo clippy-fastly && cargo clippy-axum(onlyclippy-fastlyrun locally; full matrix delegated to CI)cargo fmt --all -- --checkcd crates/trusted-server-js/lib && npx vitest run(407 passed)cd crates/trusted-server-js/lib && npm run formatcd docs && npm run formatcargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1(delegated to CI)fastly compute serveadmnow present inwindow.tsjs.bidsfor all slots. The full inline-render path (GAM awards the Prebid line item → creative renders with no PBS Cache fetch) is not yet exercised locally — mocktioneer's $0.20 bid loses to GAM's own demand, so GAM serves its own creative and the bridge never fires. Requires a winning Prebid line item in GAM to verify end-to-end.Checklist
unwrap()in production code — useexpect("should ...")tracingmacros (notprintln!) — no new Rust logging introduced