Skip to content

DEP-11967 cookie sync prebid.js integration#1

Open
IlliaMil wants to merge 19 commits intomasterfrom
DEP-11967-Cookie-Sync-Prebid.js-integration
Open

DEP-11967 cookie sync prebid.js integration#1
IlliaMil wants to merge 19 commits intomasterfrom
DEP-11967-Cookie-Sync-Prebid.js-integration

Conversation

@IlliaMil
Copy link

@IlliaMil IlliaMil commented Feb 3, 2026

✨ PR Description

Purpose: Integrate Start.io cookie sync and user ID functionality into Prebid.js adapter to enable cross-domain user identification and improve ad targeting capabilities.

Main changes:

  • Added getUserSyncs method to startioBidAdapter with iframe-based sync supporting GDPR, USP, and GPP consent parameters
  • Implemented startioSystem user ID submodule with cookie/localStorage persistence and server-side ID fetching from cs.startappnetwork.com
  • Updated documentation to replace accountId with publisherId parameter and added userSync configuration examples

Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Description using Guidelines Learn how

@coveralls
Copy link

coveralls commented Feb 3, 2026

Pull Request Test Coverage Report for Build 21868360658

Details

  • 190 of 194 (97.94%) changed or added relevant lines in 4 files are covered.
  • 5 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.002%) to 93.38%

Changes Missing Coverage Covered Lines Changed/Added Lines %
modules/startioSystem.js 39 43 90.7%
Files with Coverage Reduction New Missed Lines %
modules/excoBidAdapter.js 1 70.34%
libraries/connectionInfo/connectionUtils.js 2 42.86%
test/spec/modules/teadsBidAdapter_spec.js 2 95.63%
Totals Coverage Status
Change from base Build 21633563070: 0.002%
Covered Lines: 209357
Relevant Lines: 217576

💛 - Coveralls

@linearb linearb bot added the 20 min review label Feb 5, 2026
Copy link

@linearb linearb bot left a comment

Choose a reason for hiding this comment

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

✨ PR Review

The PR adds cookie sync and user ID integration for Start.io. The getUserSyncs implementation in the bid adapter looks solid with proper consent handling. However, there's a functional bug in the user ID module where the config parameter is completely ignored.

1 issues detected:

🐞 Bug - The config parameter is passed but never accessed, making endpoint configuration impossible despite documented support. 🛠️

Details: The getId function receives a config parameter but never uses it. The function always makes requests to the hardcoded DEFAULT_ENDPOINT instead of using config.params.endpoint. This prevents users from configuring custom endpoints even though the documentation (startioSystem.md line 42) implies endpoints should be configurable. The test on line 122 of the test file expects request.url to match validConfig.params.endpoint, but it only passes because validConfig.params.endpoint happens to equal DEFAULT_ENDPOINT.
File: modules/startioSystem.js (47-47)
🛠️ A suggested code correction is included in the review comments.

Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how

Copy link

@linearb linearb bot left a comment

Choose a reason for hiding this comment

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

✨ PR Review

The PR adds cookie sync and user ID functionality for Start.io integration. The implementation is generally sound with proper consent handling and comprehensive test coverage. However, one maintainability concern was identified regarding production code practices.

1 issues detected:

🧹 Maintainability - Exposing internal spec to global window object without environment gating creates maintainability and potential conflict issues in production 🛠️

Details: The adapter exposes its internal spec object to the global window scope in production code. While the comment indicates this is for testing purposes, exposing internal implementation details globally can lead to namespace conflicts, unintended mutations, and tight coupling between internal implementation and external code.
File: modules/startioBidAdapter.js (177-178)
🛠️ A suggested code correction is included in the review comments.

Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how

Copy link

@linearb linearb bot left a comment

Choose a reason for hiding this comment

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

✨ PR Review

The PR adds cookie sync functionality and a new user ID system. The implementation has a critical security issue with cookie configuration and a significant documentation/code mismatch that will affect users.

3 issues detected:

🔒 Security - SameSite=None cookies without Secure flag are rejected by modern browsers 🛠️

Details: Setting a cookie with SameSite=None requires the Secure flag in modern browsers (Chrome 80+, Firefox 69+, Safari 13+). Without it, the cookie will be rejected in cross-site contexts, breaking the core user syncing functionality. This affects the primary purpose of the module.
File: modules/startioSystem.js (43-43)
🛠️ A suggested code correction is included in the review comments.

🐞 Bug - Server response format in documentation doesn't match what the code actually expects

Details: The code expects server responses to contain a 'uid' field (line 59), but the documentation (startioSystem.md lines 42-44) specifies the response should have an 'id' field. Users implementing servers based on the documentation will encounter failures as the module will log an error about missing 'uid' field.
File: modules/startioSystem.js

🧹 Maintainability - Storing data in localStorage that is never read or used 🛠️

Details: The code stores 'startioId_last' in localStorage but never reads or uses this value anywhere in the codebase. This creates unnecessary storage overhead and potential confusion about the data's purpose.
File: modules/startioSystem.js (48-48)
🛠️ A suggested code correction is included in the review comments.

Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how

Copy link

@linearb linearb bot left a comment

Choose a reason for hiding this comment

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

✨ PR Review

The PR adds cookie sync functionality and a new ID system. The implementation is mostly solid, but there's a critical documentation error that would cause integration failures.

1 issues detected:

🧹 Maintainability - Documentation describes incorrect JSON field name (`id` instead of `uid`) for server response, causing integration failures. 🛠️

Details: The documentation states the server endpoint must return a JSON response with an id field (lines 39, 43), but the actual code implementation expects a uid field (startioSystem.js line 59). This mismatch would cause integrations following the documentation to fail - the module would log "Server response missing 'uid' field" errors and not store the identifier, breaking the user ID functionality entirely.
File: modules/startioSystem.md (39-47)
🛠️ A suggested code correction is included in the review comments.

Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how

Copy link

@linearb linearb bot left a comment

Choose a reason for hiding this comment

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

✨ PR Review

The PR adds cookie sync functionality and a user ID submodule for Start.io. The implementation is generally solid, but there's a significant discrepancy between documented and actual expiration behavior that needs to be addressed.

1 issues detected:

🐞 Bug - Documentation promises configurable expiration (storage.expires, default 365 days) but code hardcodes 90 days and ignores the config parameter. 🛠️

Details: The code hardcodes ID expiration to 90 days (line 39), but the documentation (startioSystem.md line 60) explicitly states that cookie expiration is set to storage.expires days with a default of 365 days. This creates a significant mismatch where users expecting 365-day persistence will only get 90 days, and the documented storage.expires configuration parameter is completely ignored.
File: modules/startioSystem.js (39-39)
🛠️ A suggested code correction is included in the review comments.

Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how

@github-actions
Copy link

Tread carefully! This PR adds 1 linter error (possibly disabled through directives):

  • modules/startioSystem.js (+1 error)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants