Skip to content

Prevent competing GPT first impressions and resize PUC shells - #1079

Open
ChristianPavilonis wants to merge 3 commits into
esi-edge-terminated-auth-mainfrom
fix/gpt-first-impression-aps-shell
Open

Prevent competing GPT first impressions and resize PUC shells#1079
ChristianPavilonis wants to merge 3 commits into
esi-edge-terminated-auth-mainfrom
fix/gpt-first-impression-aps-shell

Conversation

@ChristianPavilonis

@ChristianPavilonis ChristianPavilonis commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Stack

This child PR depends on #1070 and uses esi-edge-terminated-auth-main as its base. The parent PR's ESI and authentication changes are unchanged.

Closes #1078.

What changed

The first valid claimant now owns each physical slot's first impression for the current navigation. A publisher requestBids() call, GPT request, or GPT render prevents delayed page-bids data from retargeting and refreshing that slot. If Trusted Server claims first, the Prebid refresh wrapper filters one correlated losing publisher delivery, restores the TS targeting snapshot, and then allows later publisher refresh auctions. SPA navigation clears prior TS-managed targeting synchronously before the generation advances, so a publisher claim while page-bids is pending cannot preserve stale route keys.

The ownership state is bounded by navigation generation, exact DOM identity, auction token, and expiry. GPT and Prebid share one exact/prefix slot resolver. Overlapping auctions keep separate registrations, and exact ad-ID delivery consumes only its matching registration. An abandoned publisher claim gets at most one per-slot TS fallback after the lease expires. Publisher tokens that become TS-owned remain bounded suppression tombstones until navigation or exact-element replacement, so arbitrarily late correlated callbacks still fail closed. The bootstrap and full GPT bundle use the same identity, pruning, and slot-cap contract, and ownership listeners do not depend on diagnostics.

The Universal Creative bridge now replaces the guarded resize that it suppresses in Prebid. After a TS response is posted successfully, it expands only the authenticated source iframe and collapsed ancestors through the authenticated slot root from 1x1 to the validated winning dimensions. When a configured prefix remains ambiguous by visibility and geometry, exactly one requesting-frame match can disambiguate delivery; zero or multiple matches fail closed. The resize guard rejects ambiguous or stale sources, invalid dimensions, anchors, interstitials, fixed or sticky shells, and already-expanded frames. It covers server APS, client-side APS capabilities, inline adm, and PBS Cache responses. publisher_native keeps its separate replacement path.

The design and APS guide now document first-claimant ownership, one-shot losing-delivery suppression, authenticated frame resolution, and collapsed ancestor-chain resizing. Strict TS-first delivery remains a separate design choice.

Validation

  • cargo fmt --all -- --check
  • cargo clippy-fastly && cargo clippy-axum && cargo clippy-cloudflare && cargo clippy-cloudflare-wasm && cargo clippy-spin-native && cargo clippy-spin-wasm
  • cargo test-fastly && cargo test-axum && cargo test-cloudflare && cargo test-spin
  • cargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --test parity
  • cd crates/trusted-server-js/lib && npx vitest run (938 passed)
  • cd crates/trusted-server-js/lib && npm run lint && npm run format && node build-all.mjs
  • cd docs && npm run format
  • cd crates/trusted-server-integration-tests/browser && npx playwright test tests/shared/aps-renderer.spec.ts --project=chromium (5 passed, including real Prebid Universal Creative geometry and terminal render evidence)

Generated TSJS distribution files remain ignored and are not committed.

Residual risks and rollback

The five-second lease intentionally bounds ordinary publisher ownership and fallback timing. Publisher flows that do not request or render within that interval can yield the untouched slot to the one-shot TS fallback. Once Trusted Server owns the slot, already-registered losing tokens remain only until navigation or exact-element replacement so late correlated callbacks cannot escape suppression. The shell resize declines unusual or ambiguous GAM wrapper layouts rather than mutating them.

Rollback does not require a merge or deployment from this PR. Operators can disable the server-side opportunity path with creative_opportunities.enabled = false while reverting the child change. No stack PR is merged by this submission.

@ChristianPavilonis ChristianPavilonis changed the title Arbitrate GPT first impressions and resize PUC shells Prevent competing GPT first impressions and resize PUC shells Aug 26, 2026
Comment thread crates/trusted-server-core/src/integrations/gpt_bootstrap.js Fixed
ChristianPavilonis added a commit that referenced this pull request Aug 26, 2026
@aram356 aram356 assigned aram356 and ChristianPavilonis and unassigned aram356 Aug 27, 2026

@jevansnyc jevansnyc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I traced the initial SSAT/page-bids path, publisher requestBids path, synthetic refresh path, and Universal Creative response path. The direct single-slot case works. The inline comments cover cases that can replace or suppress the wrong impression, or leave the creative clipped at 1x1.

Comment thread crates/trusted-server-js/lib/src/integrations/prebid/index.ts
Comment thread crates/trusted-server-js/lib/src/integrations/prebid/index.ts
Comment thread crates/trusted-server-js/lib/src/integrations/prebid/index.ts
Comment thread crates/trusted-server-js/lib/src/integrations/prebid/index.ts
Comment thread crates/trusted-server-js/lib/src/core/first_impression.ts
Comment thread crates/trusted-server-js/lib/src/integrations/gpt/index.ts
Comment thread crates/trusted-server-js/lib/src/integrations/gpt/index.ts Outdated

@prk-Jr prk-Jr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

First-impression arbitration across the bootstrap, GPT bundle, and Prebid refresh wrapper, plus authenticated collapsed-shell resizing in the Universal Creative bridge. The ownership model (navigation generation + exact DOM identity + auction token + expiry) is coherent and the resize guard set is thorough. Two blocking issues: the new ownership guard suppresses the stale-targeting sweep for publisher claims, and the render bridge silently lost frame-identity disambiguation for prefix-configured div IDs.

2 of the inline comments below carry a one-click GitHub suggestion — use Commit suggestion (or Add suggestion to batch) to apply them as commits on the PR branch. The remaining comments describe the fix in prose because the change spans several call sites or needs a matching test change and cannot be auto-applied.

Blocking

🔧 wrench

  • Stale TS targeting survives on publisher-claimed slots — see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1270
  • Render bridge lost frame-identity disambiguation for prefix div IDs — see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:262

Non-blocking

🤔 thinking / ♻️ refactor / ⛏ nitpick / 📝 note

  • Two divergent prefix resolvers can put the two claimants on different elements — see inline at crates/trusted-server-js/lib/src/core/first_impression.ts:100
  • Overlap isolation is only half-realized — see inline at crates/trusted-server-js/lib/src/integrations/prebid/index.ts:1040
  • Flipped disableInitialLoad assertions are now vacuous — see inline at crates/trusted-server-js/lib/test/integrations/gpt/ad_init.test.ts:1964
  • Three independent 5000 ms constants, no cross-reference — see inline at crates/trusted-server-core/src/integrations/gpt_bootstrap.js:105
  • Resolver forces style recalc in the requestBids() hot path — see inline at crates/trusted-server-js/lib/src/core/first_impression.ts:105
  • Bootstrap and bundle prune rules differ — see inline at crates/trusted-server-js/lib/src/core/first_impression.ts:59

👍 praise

  • Collapsed-shell resize guard set and its tests — see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:283

Verification performed for this review

Both suggestions were applied in an isolated reviewer worktree at this head and checked in isolation and as a batch:

  • prettier --check src/integrations/gpt/index.ts — clean
  • npx vitest run — 901/901 passed
  • node build-all.mjs — 13 modules built
  • cargo check-fastly — clean
  • cargo fmt --all -- --check — clean
  • cargo test -p trusted-server-core --target aarch64-apple-darwin integrations::gpt — 47/47 passed
  • Post-verify patch snapshots byte-identical before and after every verification run

CI Status

  • prepare integration artifacts: PASS
  • integration tests: PASS
  • integration tests (Fastly EC lifecycle): PASS
  • browser integration tests: PASS
  • cargo test: PASS (required)
  • 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
  • cargo fmt: PASS (required)
  • vitest: PASS
  • format-typescript: PASS (required)
  • format-docs: PASS (required)
  • CodeQL: PASS
  • Analyze (rust): PASS
  • Analyze (javascript-typescript): PASS
  • Analyze (actions): PASS

Comment thread crates/trusted-server-js/lib/src/integrations/gpt/index.ts
Comment thread crates/trusted-server-js/lib/src/integrations/gpt/index.ts
Comment thread crates/trusted-server-js/lib/src/core/first_impression.ts
Comment thread crates/trusted-server-js/lib/src/integrations/prebid/index.ts Outdated
Comment thread crates/trusted-server-js/lib/test/integrations/gpt/ad_init.test.ts
Comment thread crates/trusted-server-core/src/integrations/gpt_bootstrap.js
Comment thread crates/trusted-server-js/lib/src/core/first_impression.ts Outdated
Comment thread crates/trusted-server-js/lib/src/core/first_impression.ts Outdated
Comment thread crates/trusted-server-js/lib/src/integrations/gpt/index.ts Outdated
prk-Jr and others added 2 commits August 27, 2026 20:57
* docs: plan PR 1079 review remediation

* fix(js): scope first impression delivery ownership

* fix(js): reject stale creatives and expand nested shells

* Prevent delayed publisher refresh overwrites
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TS adInit overwrites publisher ads and APS replacements remain 1x1

4 participants