pgamdirect: add getUserSyncs for cookie-sync pixel support#14777
Merged
Conversation
Follow-up to prebid#14763. Our bid adapter previously declared no user- sync pixels, which blocked cookie-based retargeting DSPs from matching our users to their own — they need the browser-side sync step to correlate, and without it their bids come back heavily suppressed for "user not matched." Implementation routes sync URLs through the OpenRTB response from our bidder at `response.ext.cookies[]`. Each entry carries: { type: 'image' | 'iframe', url: string } The per-DSP sync URL list lives server-side (one entry per downstream DSP we integrate), so this adapter stays stable even as we add / rotate DSPs. Behaviour: - Empty serverResponses or no ext.cookies → return [] - Respects syncOptions.iframeEnabled / pixelEnabled (publisher- controlled — some publishers disallow iframe syncs) - Drops malformed entries (missing url or unknown type) - Caps at 5 pixels per response to avoid cookie floods Consent note: Prebid passes parsed GDPR / USP / GPP state, but we don't filter server-side. Each DSP's sync URL already encodes its own consent handling (appending ?gdpr=1&gdpr_consent=... as each requires). Future revision can add per-DSP consent framework declarations + server-side suppression if a DSP fails to register handling for the caller's framework. Tests: +6 spec cases covering empty, malformed, caps, and consent- driven filtering. 32/32 pass (was 26).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31a81c3d52
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // limit but more than a handful of syncs per page visit is | ||
| // both wasteful (cookie race conditions in the browser) and | ||
| // suspicious to auditors. | ||
| if (out.length >= 5) break; |
There was a problem hiding this comment.
The hard stop at 5 syncs here bypasses Prebid’s existing userSync.syncsPerBidder control and forces a lower limit even when publishers explicitly raise it (or set it to 0 for unlimited). In this codebase, src/userSync.ts already enforces per-bidder limits during registerSync, so this extra cap will silently drop valid sync URLs once ext.cookies has more than five entries, reducing match coverage for downstream DSPs despite publisher configuration.
Useful? React with 👍 / 👎.
Codex pointed out on prebid#14777 that the hard-cap at 5 pixels bypassed Prebid's `userSync.syncsPerBidder` control in src/userSync.ts. Core already enforces the per-bidder sync limit based on publisher configuration; an additional cap inside the adapter silently drops valid sync URLs when a publisher raises the limit (or sets 0 = unlimited), reducing match coverage for downstream DSPs. Fix: remove the break on length >= 5 so getUserSyncs returns the full filtered list. Prebid core does the right thing from here. Updated the corresponding test to assert the full list comes back with a comment pointing to where the authoritative cap lives. 32/32 tests still pass.
Collaborator
Coverage Report for CI Build 24856585461Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage increased (+33.2%) to 96.409%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions38 previously-covered lines in 2 files lost coverage.
Coverage Stats
💛 - Coveralls |
Contributor
Author
|
Note: the review on commit 31a81c3 looks at the pre-fix version. The 5-pixel cap you flagged was removed in commit 29ddd29 (pushed shortly after the initial open). Current HEAD returns the full filtered list and relies on Prebid core's |
patmmccann
approved these changes
Apr 23, 2026
oleksandrhn-code
pushed a commit
to DimaIntentIQ/Prebid.js
that referenced
this pull request
Apr 28, 2026
) * pgamdirect: add getUserSyncs for cookie-based retargeting DSPs Follow-up to prebid#14763. Our bid adapter previously declared no user- sync pixels, which blocked cookie-based retargeting DSPs from matching our users to their own — they need the browser-side sync step to correlate, and without it their bids come back heavily suppressed for "user not matched." Implementation routes sync URLs through the OpenRTB response from our bidder at `response.ext.cookies[]`. Each entry carries: { type: 'image' | 'iframe', url: string } The per-DSP sync URL list lives server-side (one entry per downstream DSP we integrate), so this adapter stays stable even as we add / rotate DSPs. Behaviour: - Empty serverResponses or no ext.cookies → return [] - Respects syncOptions.iframeEnabled / pixelEnabled (publisher- controlled — some publishers disallow iframe syncs) - Drops malformed entries (missing url or unknown type) - Caps at 5 pixels per response to avoid cookie floods Consent note: Prebid passes parsed GDPR / USP / GPP state, but we don't filter server-side. Each DSP's sync URL already encodes its own consent handling (appending ?gdpr=1&gdpr_consent=... as each requires). Future revision can add per-DSP consent framework declarations + server-side suppression if a DSP fails to register handling for the caller's framework. Tests: +6 spec cases covering empty, malformed, caps, and consent- driven filtering. 32/32 pass (was 26). * pgamdirect: remove in-adapter sync cap (address Codex review) Codex pointed out on prebid#14777 that the hard-cap at 5 pixels bypassed Prebid's `userSync.syncsPerBidder` control in src/userSync.ts. Core already enforces the per-bidder sync limit based on publisher configuration; an additional cap inside the adapter silently drops valid sync URLs when a publisher raises the limit (or sets 0 = unlimited), reducing match coverage for downstream DSPs. Fix: remove the break on length >= 5 so getUserSyncs returns the full filtered list. Prebid core does the right thing from here. Updated the corresponding test to assert the full list comes back with a comment pointing to where the authoritative cap lives. 32/32 tests still pass. --------- Co-authored-by: Patrick McCann <patmmccann@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #14763. Adds
getUserSyncsto the pgamdirect adapter so cookie-based retargeting DSPs can match our users to theirs via the standard browser-side sync flow.Previously our adapter returned no syncs, which caused those DSPs to come back with heavily suppressed bids ("user not matched"). This PR routes sync URLs through the OpenRTB response at `response.body.ext.cookies[]` — one entry per downstream DSP — so the list stays server-driven and the adapter stays stable as we add/rotate DSPs.
Behaviour
Wire format
Each cookie entry: `{ type: 'image' | 'iframe', url: string }`
Consent handling
GDPR / USP / GPP parsed state is passed through but not filtered server-side in this revision — each DSP's sync URL already encodes its own consent handling (appending `?gdpr=1&gdpr_consent=...` as each requires). A future revision can add per-DSP consent framework declarations + server-side suppression if any DSP fails to register handling.
Tests
6 new spec cases: empty responses, malformed entries, size cap, iframe-only publishers, image-only publishers, all-disabled. 32/32 total tests pass.
Test plan
🤖 Generated with Claude Code