Skip to content

Support analytics adapters in generated Prebid bundles - #1090

Open
ChristianPavilonis wants to merge 10 commits into
mainfrom
1085-prebid-module-map
Open

ChristianPavilonis wants to merge 10 commits into
mainfrom
1085-prebid-module-map

Conversation

@ChristianPavilonis

Copy link
Copy Markdown
Collaborator

Summary

  • Adds analytics adapter selection to generated Prebid bundles so publisher calls to pbjs.enableAnalytics can use adapters such as ATS without missing-registry errors.
  • Replaces the separate bidder and User ID settings with one typed module map covering bidder, User ID, and analytics modules.
  • Validates every selected module against the locked Prebid.js package before generating source or invoking Vite, then records module stems and registered runtime codes separately.

Changes

File Change
crates/trusted-server-cli/src/prebid_bundle.rs Parses the typed module map, rejects removed fields, sends one JSON request to the generator, and validates the versioned manifest before updating bundle hashes.
crates/trusted-server-js/lib/build-prebid-external.mjs Validates locked-package metadata and exports, generates selected module imports, and writes nested versioned manifests.
crates/trusted-server-js/lib/src/integrations/prebid/index.ts Reads the nested browser manifest and uses registered bidder codes for runtime diagnostics.
crates/trusted-server-js/lib/src/integrations/prebid/user_id_modules.json Keeps User ID registry metadata while removing generator-controlled import paths.
crates/trusted-server-js/lib/src/integrations/prebid/user_id_modules.ts Removes the obsolete importPath field from the registry type.
crates/trusted-server-js/lib/test/build-prebid-external.test.mjs Covers request parsing, package validation, metadata and export containment, generated imports, and manifest output.
crates/trusted-server-js/lib/test/integrations/prebid/index.test.ts Covers nested manifest parsing and independent bidder and User ID diagnostics.
crates/trusted-server-js/lib/test/prebid-artifact-integration.test.mjs Proves ATS registration, publisher queue enablement, auction behavior, network isolation, omitted analytics, and watchdog queue draining with production artifacts.
docs/guide/cli.md Documents the typed module configuration and generated manifest workflow.
docs/guide/integrations/prebid.md Documents exact module stems, analytics provider ownership, ATS configuration, and upstream-only restrictions.
docs/superpowers/plans/2026-08-28-prebid-bundle-module-map.md Records the reviewed implementation and validation plan.
docs/superpowers/specs/2026-08-28-prebid-bundle-module-map-design.md Records the implemented configuration, validation, manifest, and runtime design.
trusted-server.example.toml Updates the sample Prebid bundle configuration to use the typed module map.

Scope

This pull request changes the Rust command-line interface, Node.js bundle generator, browser diagnostics, tests, and operator documentation because they share one configuration and manifest contract. It is limited to modules shipped by the locked Prebid.js package. It does not add custom module paths, download third-party adapters, or move pbjs.enableAnalytics options out of publisher code.

Closes

Closes #1085

Test plan

  • cargo test-fastly && cargo test-axum
  • cargo clippy-fastly && cargo clippy-axum
  • cargo fmt --all -- --check
  • JS tests: cd crates/trusted-server-js/lib && npx vitest run
  • JS format: cd crates/trusted-server-js/lib && npm run format
  • Docs format: cd docs && npm run format
  • WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
  • Manual testing via fastly compute serve
  • Other: cargo test-cloudflare, cargo test-spin, all six target-matched Clippy commands, parity tests, CLI tests, JavaScript build, documentation build, a real bundle smoke test, and the unsupported-module failure path.

Checklist

  • Changes follow CLAUDE.md conventions
  • No unwrap() in production code; new fallible paths return actionable errors
  • No runtime logging was added; existing project logging conventions are unchanged
  • New code has tests
  • No secrets or credentials committed

@ChristianPavilonis ChristianPavilonis self-assigned this Aug 28, 2026
Comment thread crates/trusted-server-js/lib/test/build-prebid-external.test.mjs Fixed
@aram356 aram356 added this to the 202609 milestone Aug 29, 2026
@ChristianPavilonis
ChristianPavilonis marked this pull request as draft August 31, 2026 15:29
@ChristianPavilonis
ChristianPavilonis marked this pull request as ready for review September 9, 2026 17:39

@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

Typed module map, versioned manifest, and analytics support for ts prebid bundle. The resolver hardening is the strongest part of this change and I have no correctness objections to the implementation. Everything below is non-blocking: one documentation gap on the operator upgrade path, one test assertion that is weaker than the PR description claims, two maintainability items, and one follow-up that is out of scope for this PR.

3 of the inline comments below carry a one-click GitHub suggestion — use Commit suggestion (or Add suggestion to batch) to apply them. Each was applied to a scratch worktree at this head and verified before posting: npx prettier --check, npx vitest run test/prebid-artifact-integration.test.mjs (5/5), and docs prettier via the pinned docs/node_modules binary. The three together produce no drift beyond the approved ranges. The remaining comments are prose because the fix spans multiple hunks or files.

Non-blocking

🤔 thinking

  • Operator upgrade path is not written down anywhere but a CLI error string — see inline at docs/guide/integrations/prebid.md:185

♻️ refactor

  • expectNoUnexpectedNetworkActivity never asserts on stubs.requests — see inline at crates/trusted-server-js/lib/test/prebid-artifact-integration.test.mjs:198
  • Metadata directory is re-read once per selected module — see Cross-cutting below
  • Pinned Prebid version hardcoded in three assertions — see Cross-cutting below

⛏ nitpick

  • Test name describes one artifact, body asserts both — see inline at crates/trusted-server-js/lib/test/prebid-artifact-integration.test.mjs:417

📌 out of scope

  • Analytics adapters compile into a bundle with no TCF/GPP purpose enforcement — see Cross-cutting below

👍 praise

  • Resolver hardening — see Cross-cutting below

Cross-cutting / body-level findings

  • ♻️ Metadata directory is re-read once per selected moduleresolveOneModule calls fs.readdirSync(context.metadataDir) inside the per-module loop (crates/trusted-server-js/lib/build-prebid-external.mjs:307), and that directory holds 665 entries in the pinned package. The readdir itself is the right call — it is an exact-case membership test, which fs.existsSync would not give you on a case-insensitive filesystem, and the ATSanAlyticsAdapter test depends on that behaviour — it just does not need repeating per module. Hoist it into resolveBundleModules beside the canonicalMetadataDir validation and thread a Set through context:

    // resolveBundleModules, after canonicalMetadataDir is validated:
    const metadataEntries = new Set(fs.readdirSync(canonicalMetadataDir));
    // ...include metadataEntries in the context object passed to resolveOneModule
    
    // resolveOneModule:
    if (!context.metadataEntries.has(metadataFilename)) {
      throw unsupportedModuleError(definition, stem, context.prebidVersion);
    }

    Apply manually — this touches two non-adjacent hunks, so it cannot be a single suggestion.

  • ♻️ Pinned Prebid version hardcoded in three assertions that are not about the versioncrates/trusted-server-js/lib/test/build-prebid-external.test.mjs:52, :671, :739, and crates/trusted-server-js/lib/test/prebid-artifact-integration.test.mjs:279 all pin 10.26.0. A prebid.js bump breaks four assertions whose subject is module resolution, not the version number. verifyPrebidPackageVersion is already exported and already imported in the first file — deriving it once at module scope (const prebidVersion = verifyPrebidPackageVersion();) and substituting keeps the version-interpolation assertion at :671 meaningful while removing the coupling. The writeVersionFixtures literals are correct as they are; those are the subject of their own tests.

    Apply manually — spans two files.

  • 📌 Analytics adapters now compile into a bundle with no TCF/GPP purpose enforcementrenderExternalEntry imports consentManagementTcf, consentManagementGpp, and consentManagementUsp (crates/trusted-server-js/lib/build-prebid-external.mjs:526-528) but not tcfControl or gppControl. The consentManagement* modules read and expose the consent string; the *Control modules are what actually gate vendor activity on denied purposes. That gap predates this PR, but this PR is what makes it straightforward to compile an identity-analytics vendor into the bundle. Publisher JavaScript owns enableAnalytics, so a publisher can gate it themselves — but the operator has no way to add the enforcement modules through bundle.modules, since they are not bidder, userId, or analytics component types. I could not find an open issue tracking this. Worth filing one before analytics ships to a GDPR publisher; no change requested here.

  • 👍 Resolver hardeningrealpath containment checks on both the metadata file and the resolved package export, with a symlink-escape test for each; exact-case metadata matching; the lockfile-versus-installed version check with actionable npm ci guidance; and the removal of the old silent fallback that stamped the module stem as a bidder code when metadata was missing. Asserting createGeneratedPaths and buildBundle were not.toHaveBeenCalled() on every failure path is a precise way to pin the "fail before Vite" ordering, and it is done consistently across six tests.

CI Status

  • integration tests (Fastly EC lifecycle): PASS
  • integration tests: PASS
  • browser integration tests: PASS
  • CodeQL: PASS
  • Analyze (javascript-typescript): PASS
  • cargo test (ts CLI, native): PASS
  • format-docs: FAIL (required)
  • format-typescript: PASS (required)
  • Analyze (rust): PASS
  • cargo test (axum native): PASS
  • cargo test: PASS (required)
  • cargo check (cloudflare native + wasm32-unknown-unknown): PASS
  • cargo check/build/test (spin native + wasm32-wasip1): PASS
  • cargo test (cross-adapter parity): PASS
  • prepare integration artifacts: PASS
  • vitest: PASS
  • Analyze (actions): PASS
  • cargo fmt: PASS (required)

format-docs is two whitespace-only deviations in docs/guide/integrations/prebid.md, both introduced by 1e52d790: a missing blank line after the ### Browser configuration options heading (line 79), and a doubled blank line before ## External Bundle Generation (line 144). Reviewer confirmed those are the only two, and that fixing them makes cd docs && npm run format pass. They were deliberately excluded from this review's findings at the reviewer's request, but format-docs is a required check, so merge stays blocked until they are fixed.

Comment thread docs/guide/integrations/prebid.md
Comment thread crates/trusted-server-js/lib/test/prebid-artifact-integration.test.mjs Outdated

@aram356 aram356 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

Typed module map, upstream-only resolution, and the schema-versioned manifest are coherent across the CLI, generator, and shim, and the test coverage is thorough. Two blocking problems: the core runtime config still models the old bundle.adapters shape, so the config this PR documents cannot be validated, pushed, or loaded; and the required format-docs check fails on docs/guide/integrations/prebid.md.

4 of the inline comments below carry a one-click GitHub suggestion — use Commit suggestion (or Add suggestion to batch for several at once) to apply them as commits on the PR branch. The remaining comments describe the fix in prose because the change touches other files or lines outside the diff and can't be auto-applied.

Blocking

🔧 wrench

  • Core runtime config still rejects bundle.modules — see inline at crates/trusted-server-cli/src/prebid_bundle.rs:67
  • format-docs (required) fails: missing blank line after heading — see inline at docs/guide/integrations/prebid.md:79
  • format-docs (required) fails: extra blank line before heading — see inline at docs/guide/integrations/prebid.md:144

Non-blocking

⛏ nitpick

  • Version-specific analytics claim will rot — see inline at docs/guide/integrations/prebid.md:564
  • Unrelated override-rules example added to the config template — see inline at trusted-server.example.toml:470
  • Metadata directory re-read per selected stem — see inline at crates/trusted-server-js/lib/build-prebid-external.mjs:307

Cross-cutting / body-level findings

  • 🌱 Shim parses runtimeCodes.analytics but never uses itgetExternalBundleManifest() validates and types the analytics runtime codes, then nothing reads them. The bidder codes drive the client_side_bidders check; the analytics codes could drive an equivalent one. A follow-up could wrap pbjs.enableAnalytics and log an operator-facing error naming [integrations.prebid.bundle.modules].analytics when the requested provider is not in the manifest, since Prebid's own no analytics adapter found in registry message does not mention the config key that fixes it.

CI Status

  • integration tests (Fastly EC lifecycle): PASS
  • integration tests: PASS
  • browser integration tests: PASS
  • CodeQL: PASS
  • Analyze (javascript-typescript): PASS
  • cargo test (ts CLI, native): PASS
  • format-docs: FAIL (required)
  • format-typescript: PASS (required)
  • Analyze (rust): PASS
  • cargo test (axum native): PASS
  • cargo test: PASS (required)
  • cargo check (cloudflare native + wasm32-unknown-unknown): PASS
  • cargo check/build/test (spin native + wasm32-wasip1): PASS
  • cargo test (cross-adapter parity): PASS
  • prepare integration artifacts: PASS
  • vitest: PASS
  • Analyze (actions): PASS
  • cargo fmt: PASS (required)

The github-code-quality bot's fakeBundleMetadata comment is stale; that helper no longer exists at this head.

Comment thread crates/trusted-server-cli/src/prebid_bundle.rs
Comment thread docs/guide/integrations/prebid.md
Comment thread docs/guide/integrations/prebid.md Outdated
Comment thread docs/guide/integrations/prebid.md Outdated
Comment thread trusted-server.example.toml Outdated
Comment thread crates/trusted-server-js/lib/build-prebid-external.mjs Outdated
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.

Support analytics adapters in generated Prebid bundles

3 participants