Add Osano consent mirror integration#773
Conversation
4c3014b to
67cae34
Compare
aram356
left a comment
There was a problem hiding this comment.
Summary
The Osano mirror is a good direction and the PR has solid CI coverage, but I found two consent-state correctness issues that should be fixed before merge. I also included a small registry refactor request and two non-blocking follow-ups around migration/docs.
Blocking
🔧 wrench
- Overlapping mirror attempts can apply stale consent:
mirrorOsanoConsent()awaits async IAB API reads without guarding against older attempts completing after newer consent events, so a delayed callback can overwrite the current consent state. (crates/js/lib/src/integrations/osano/index.ts:394) - GPP empty-value clearing bypasses the Osano readiness guard: USP and TCF empty values wait for Osano readiness before clearing stale cookies, but GPP ready-with-empty clears immediately. (
crates/js/lib/src/integrations/osano/index.ts:290)
Non-blocking
♻️ refactor
- Derive JS module availability instead of hardcoding integration IDs:
JS_EXCLUDEDhas to stay in sync with Rust registrations and generated TSJS modules. Prefer checking whethertrusted_server_js::module_bundle(id)exists for each enabled integration. (crates/trusted-server-core/src/integrations/registry.rs:1144)
🤔 thinking
- Sourcepoint mirror is now opt-in, which is a behavior change: existing deployments that relied on the unconditional Sourcepoint GPP mirror will stop shipping that JS unless they explicitly enable
[integrations.sourcepoint]. This likely deserves a migration note. (crates/trusted-server-core/src/integrations/registry.rs:1146)
🌱 seedling
- Public integration docs should mention Osano enablement: the design spec is useful, but operators will likely look in public guide/configuration docs for
[integrations.osano] enabled = trueand the first-request/next-request limitation.
CI Status
- Analyze (actions): PASS
- Analyze (javascript-typescript): PASS
- Analyze (rust): PASS
- CodeQL: PASS
- browser integration tests: PASS
- cargo fmt: PASS
- cargo test: PASS
- format-docs: PASS
- format-typescript: PASS
- integration tests: PASS
- prepare integration artifacts: PASS
- vitest: PASS
|
Also addressed the non-inline docs feedback in e62f8e0 by adding public Osano enablement docs, linking the Osano guide from the sidebar and integrations overview, and documenting the first-request/subsequent-request limitation. |
aram356
left a comment
There was a problem hiding this comment.
Summary
Re-review. Both prior 🔧 blockers and all three non-blocking items from the earlier review are resolved, each with regression coverage, and CI is fully green.
- Stale overlapping consent — fixed via the
mirrorGenerationguard; only the latest in-flight attempt applies (mirrorOsanoConsent). Covered by a dedicated overlap test. - GPP empty-value clearing — now routes through the same Osano-readiness guard as USP/TCF (
emptyAfterOsanoReadyResult). Covered by preserve-until-ready and clear-when-ready tests. - JS module derivation —
js_module_ids()now derives availability fromtrusted_server_js::module_bundle(id)over enabled integrations instead of a hardcoded denylist. Verified behavior-preserving againstmain(only the intended Sourcepoint change). - Sourcepoint opt-in — documented as a Breaking CHANGELOG entry plus a migration note in the Sourcepoint guide.
- Public docs — added the Osano guide, configuration section, sidebar/overview links, and the first-request limitation.
I independently traced the cookie ownership/clearing state machine, the multi-attempt generation race, and the denylist→allowlist refactor — no blocking issues. Approving. Two optional non-blocking notes are left inline.
CI Status
- cargo fmt: PASS
- cargo clippy / Analyze (rust): PASS
- cargo test: PASS
- vitest: PASS
- format-typescript / format-docs: PASS
- browser + integration tests: PASS
Reconcile main (Osano CMP mirror #773, fastly extraction out of core, EC/storage refactors) with this branch's EdgeZero/fastly-0.12 bump. Resolution decisions: - Keep this branch's dependency bump: fastly/log-fastly 0.12 and edgezero git deps tracking branch=main (over main's pinned rev + 0.11.12), plus the trusted-server-* crate renames. - Take main's newer runtime code for conflicted files (EC http:: migration, settings, geo, testlight, adapter restructure), then forward-port it to the edgezero-main / fastly-0.12 API surface: - Body::into_bytes() now returns Option<Bytes>; buffered sites use unwrap_or_default() (batch_sync, pull_sync, identify/testlight tests). - fastly 0.12 get_tls_protocol()/get_tls_cipher_openssl_name() return Result<Option>; call sites use .ok().flatten(). - edgezero-main moved adapter symbols into submodules (request::into_core_request, config_store::FastlyConfigStore, context::FastlyRequestContext) and made router oneshot()/IntoResponse into_response() fallible; dispatch and test sites updated accordingly. - Accept main's deletion of core storage/secret_store.rs (fastly removal). - Fix a directory-rename gap: main added Osano JS under the old crates/js path; relocated into crates/trusted-server-js so the bundle is discovered. Verified: cargo fmt/clippy/test --workspace, wasm release build, JS vitest + format. All green.
Summary
Changes
crates/js/lib/src/integrations/osano/index.tsus_privacy,__gpp,__gpp_sid, andeuconsent-v2with_ts_consent_src=osanoownership.crates/js/lib/test/integrations/osano/index.test.tscrates/trusted-server-core/src/integrations/osano.rscrates/trusted-server-core/src/integrations/mod.rscrates/trusted-server-core/src/integrations/registry.rstrusted-server.toml[integrations.osano]config block.docs/superpowers/specs/2026-06-17-osano-consent-mirror-design.mdCloses
Closes #772
Test plan
cargo test --workspacecargo clippy --workspace --all-targets --all-features -- -D warningscargo fmt --all -- --checkcd crates/js/lib && npx vitest runcd crates/js/lib && npm run formatcd docs && npm run formatcargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1fastly compute servecd crates/js/lib && node build-all.mjscd docs && npx prettier --check .Checklist
unwrap()in production code — useexpect("should ...")println!)