Add ts CLI ad-template config diagnostics and browser audit#823
Open
prk-Jr wants to merge 160 commits into
Open
Add ts CLI ad-template config diagnostics and browser audit#823prk-Jr wants to merge 160 commits into
prk-Jr wants to merge 160 commits into
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Incorporate all review feedback (aram356 + jevansnyc): cache contract, consent/GDPR gating, async restructuring detail, CreativeOpportunityFormat schema, glob pattern fix, XSS escaping, win notifications, APS params, timeout config key, defineSlot fix, gpt.rs ownership, KV migration path, Phase 2 sketch - Fix Prettier formatting (format-docs CI) - Add implementation plan (12 tasks, TDD, ordered by dependency)
- Incorporate all review feedback (aram356 + jevansnyc): cache contract, consent/GDPR gating, async restructuring detail, CreativeOpportunityFormat schema, glob pattern fix, XSS escaping, win notifications, APS params, timeout config key, defineSlot fix, gpt.rs ownership, KV migration path, Phase 2 sketch - Fix Prettier formatting (format-docs CI) - Add implementation plan (12 tasks, TDD, ordered by dependency)
Replace the head-injected __ts_bids design with a server-cached bid delivery model fetched by the client via a new /ts-bids endpoint. The auction never blocks page rendering — </head> flushes immediately, body parses without waiting for bids, and the client fetches bids in parallel with content paint. Key changes: - §2 Goal: bid delivery decoupled from page rendering; FCP unchanged from no-TS baseline - §4.3 Auction Trigger: drop buffered/streaming dichotomy; single mode forces chunked encoding on all origins (WordPress, NextJS, etc.) - §4.4 Head Injection: only __ts_ad_slots and __ts_request_id injected at <head> open; bid results moved to /ts-bids endpoint - §4.6 Client Residual: __tsAdInit defines slots immediately, fetches bids via /ts-bids, applies targeting and fires refresh() after resolve - §4.7 (new) Caching Behavior: explicit cacheability table for HTML, JS, CSS, tsjs bundle, bid results; Fastly edge HTTP cache leveraged for origin HTML - §5 Request-Time Sequence: full mermaid diagram covering content + creative + burl flow with cache-hit and cache-miss branches; separate text sequences for cache-hit (~80ms FCP, ~900ms ad-visible) and cache-miss (~250ms FCP, ~1,050ms ad-visible) - §6 Performance Summary: cache-hit and cache-miss columns; FCP added as a tracked metric - §7 Implementation Scope: add bid_cache.rs, /ts-bids endpoint, force chunked encoding step - §8 Edge Cases: origin-agnostic entries; new entries for /ts-bids 404 and client-never-fetches-/ts-bids Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pivot from the /ts-bids fetch endpoint + in-process bid_cache design to
inline __ts_bids injection before </body>. The earlier design relied on
shared state that doesn't reliably survive Fastly Compute's per-request Wasm
isolate model — body injection achieves the same FCP property in a single
response with no shared-state requirement.
Key changes:
- §4.3: replace /ts-bids long-poll with bounded </body> hold tied to
A_deadline. Body content above </body> paints first; close-tag held
until auction completes or A_deadline fires (graceful __ts_bids = {}
fallback).
- §4.3: add auction-eligibility gating (consent, bot UA, prefetch hints,
HEAD method, slot match) so auctions fire on real first-page-load
impressions only.
- §4.4: replace __ts_request_id + /ts-bids machinery with two inline
<script> blocks — __ts_ad_slots at <head> open, __ts_bids before
</body> via lol_html el.on_end_tag().
- §4.5: move both nurl and burl to client-side firing from
slotRenderEnded after hb_adid match. Server-side firing rejected to
avoid billing inflation on bids that never render.
- §4.6: replace fetch+Promise pattern with synchronous __ts_bids read.
Add lazy slim-Prebid loader (post-window.load) for scroll/refresh
auctions and Phase B identity warm-up. Add ts_initial=1 slot-ownership
sentinel.
- §4.7: switch Cache-Control from private, no-store to private,
max-age=0 to preserve browser BFCache eligibility while still
preventing intermediate-cache leaks.
- §4.8 (new): document the EC/KV identity model as load-bearing auction
input — Phase A retrieval at request time, Phase B post-render
enrichment via slim-Prebid userID modules. Add bare-EC first-impression
caveat and auction_eid_count metric. Note federated-consortium
passphrase property and clickstream-compounding speed win.
- §5: update mermaid + cache-hit/miss timelines for bounded body hold;
ad-visible converges to ~870ms (hit) / ~1,020ms (miss).
- §6: drop /ts-bids RTT row; add DCL row; add clickstream-compounding,
TS-overhead, identity-coverage, and confidence-interval framing.
- §7: drop bid_cache.rs and /ts-bids endpoint from scope; add
auction-eligibility gating and slim-Prebid bundle build target. Add
explicit "Deleted" subsection.
- §8: drop /ts-bids edge cases; add SPA/pushState, bare-EC, bot/prefetch,
HEAD, BFCache restoration cases.
- §9.6: server-side GAM downgraded from "Phase 2 commitment" to
aspirational and contingent on Google agreement. §9.8 (slim-Prebid
bundle composition), §9.9 (Privacy Sandbox), §9.10 (per-bidder consent)
added as follow-ups.
Implementation plan at docs/superpowers/plans/2026-04-30-server-side-ad-templates.md
is now stale relative to this spec; needs regenerating before code lands.
…ities.toml Adds the creative_opportunities field to Settings struct to deserialize configuration for the server-side ad auction feature. Includes build.rs stubs for types required during build-time configuration validation. Creates creative-opportunities.toml with example slot configuration and updates trusted-server.toml with the [creative_opportunities] section defining GAM network ID, auction timeout, and price granularity settings. Tests pass with proper TOML parsing of the creative_opportunities section.
…ared auction state
- Add `ad_slots_script: Option<String>` and `ad_bids_state: Arc<RwLock<Option<String>>>` fields to `HtmlProcessorConfig`
- Update `from_settings` to initialize both new fields with safe defaults
- Prepend `ad_slots_script` inside the existing `<head>` handler before integration inserts
- Add `element!("body", ...)` handler that uses `end_tag_handlers()` to inject `__ts_bids` before `</body>`; falls back to empty `{}` when auction state is `None`
- Add `IntegrationRegistry::empty_for_tests()` test helper
- Add three new tests covering all injection paths
…gibility gates; max-age=0 - Make handle_publisher_request async; add orchestrator and slots_file params - Dispatch origin request with send_async before running auction in parallel - Gate auction on GET, no prefetch, no bot, matched slots, TCF purpose-1 consent - Run server-side auction and write bucketed bids to ad_bids_state Arc<RwLock> - Compute ad_slots_script after response headers; set Cache-Control: private, max-age=0 - Fix Stream arm to thread actual ad_slots_script and ad_bids_state through - Add build_auction_request, build_bid_map, build_bids_script, build_ad_slots_script helpers - Update route_tests.rs to pass empty slots_file to route_request
…m slotRenderEnded
- build_bid_map now returns serde_json::Map with full bid objects (hb_pb,
hb_bidder, hb_adid, nurl, burl) instead of a plain CPM string map
- build_bids_script / build_ad_slots_script now emit full <script> tags
using JSON.parse("…") for safe inline embedding; add html_escape_for_script helper
- build_ad_slots_script uses correct property names (gam_unit_path, div_id,
formats, targeting) matching the client-side TSJS bundle expectations
- Replace map_or(false, …) with is_some_and(…) on lines 546, 549, 567
- Add # Panics doc sections to handle_publisher_request and create_html_processor
…nities.toml at startup
… from slotRenderEnded; slim-Prebid lazy loader
- Enable APS and adserver_mock in auction config; set providers and mediator - Increase auction_timeout_ms from 500ms to 3000ms — 500ms was too tight for HTTPS round-trips to mocktioneer, leaving the mediator zero budget - Fix mediation request: send numeric price instead of opaque encoded_price; mocktioneer requires a decoded price field and does not support encoded_price - Expand creative-opportunities slot page_patterns to include /news/**
Define SlotRenderEndedEvent, SlotRenderEvent, and TestWindow types to eliminate all @typescript-eslint/no-explicit-any violations in gpt/index.ts and gpt/index.test.ts. Extend GptWindow with __tsjs_slim_prebid_url so installSlimPrebidLoader avoids the any cast.
Set gam_network_id to 88059007 (autoblog production network). Update atf_sidebar_ad slot to /88059007/autoblog/news with div_id ad-atf_sidebar-0-_r_2_ (desktop ATF sidebar, 300x250); restrict page_patterns to article paths only (/20**, /news/**) since that div does not exist on the homepage. Add homepage_header_ad slot targeting /88059007/autoblog/homepage with ad-header-0-_R_jpalubtak5lb_ for 970x90/728x90/970x250 leaderboard formats. Reduce auction_timeout_ms from 3000 to 500 to cap TTFB at the spec-recommended ceiling.
The bids script set window.__ts_bids but never invoked the __tsAdInit function, leaving GPT slots undefined and server-side targeting (hb_pb, hb_bidder) never applied. Both the winning-bid path (build_bids_script) and the no-auction fallback (html_processor None branch) now guard-call the function after the assignment.
Blocking: - Move tokio to [dev-dependencies] in trusted-server-core; it was only used by #[tokio::test] and was linking the runtime into the wasm prod build. Confirmed the release wasm adapter build no longer pulls tokio. - Roll back the SPA currentPath on a failed /__ts/page-bids fetch so a transient error no longer permanently strands that route (gpt/index.ts). Build/runtime parity and diagnostics: - Bound creative-opportunity format width/height to u32 range at build time so values the runtime u32 cannot hold are rejected early. - Add #[serde(deny_unknown_fields)] to the build.rs config stub to match the runtime type and reject mistyped table keys at build time. - Warn when the <body> end-tag handler is absent so a silently non-rendering server-side ad feature is diagnosable. - Log dropped slot bidders that are neither configured nor the aps provider. - Log build_bid_index collisions (multiple bids per seat/imp). JS correctness: - Narrow uid.atype to a number before the range check in sanitizeAuctionUid. - Resolve findInjectedSlotForRefresh by exact/container match before the prefix fallback, with a regression test for prefix-overlapping div_ids. - Guard the gpt_bootstrap prefix scan against an empty div_id. - Route injectAdmIntoSlot through findSlotElementByDivId for consistency. Cleanup and docs: - Remove the dead has_post_processors routing dependency from classify_response_route and (now unused) handle_publisher_request. - Extract the duplicated EID resolution/consent-gating/device tail shared by the initial-page and page-bids dispatch paths into one helper. - Anchor the surrogate cache-header list in a shared const so the legacy and EdgeZero Set-Cookie privacy paths stay aligned. - Refresh stale docs (PublisherResponse::Stream, the publisher module platform-coupling note, and UserInfo.eids consent-gate location).
Moving tokio to trusted-server-core dev-dependencies removed it from the crate's normal dependency list, so the integration-tests lockfile (which resolves core's non-dev deps) no longer pins tokio under core. Keeps `cargo --locked` green for the integration job.
These EdgeZero-style adapters finalize buffered, and the sync
`buffer_publisher_response` drives `stream_publisher_body`, which ignores
`params.dispatched_auction` — so they injected an empty `tsjs.bids = {}`
while Fastly (legacy streaming finalize) served real bids.
- Add `buffer_publisher_response_async` in core: for the Stream variant it
drives `stream_publisher_body_async`, which awaits
`collect_dispatched_auction`, writes `ad_bids_state`, and injects the bids
before `</body>`.
- Pass the configured `creative_opportunities.slot` (not empty) to
`handle_publisher_request` on all three adapters; it matches them against
the request path internally.
- Call the async finalize from each adapter (Cloudflare/Spin via their
now-async `resolve_publisher_response`).
EID targeting stays off for now (these adapters pass `kv: None`).
…ters The auction consent gate (`consent_allows_server_side_auction`) reads jurisdiction and TCF consent from the EC context. The adapters passed `EcContext::default()` to `handle_publisher_request`, leaving jurisdiction Unknown with no consent — so the gate failed closed and no auction ran (empty `tsjs.bids`), even though the slots matched. Build the context via `read_from_request_with_geo` (consent from the request, geo from the platform), mirroring the Fastly entry point, and fall back to default on a parse error. Cloudflare resolves geo from the Workers `cf` object when deployed; Axum and Spin have no-op geo providers, so on those a known non-GDPR jurisdiction requires the request to carry geo or the gate needs a TCF consent signal.
When the auction does not run, this pinpoints which gate suppressed it (slots, bot, navigation, consent, or orchestrator kill switch) instead of only seeing `dispatch_auction: None`. Pair with the EC-context jurisdiction log when consent_allows_auction is false.
The EdgeZero buffered path passed empty slots and finalized via the sync `buffer_publisher_response`, so configured creative-opportunity slots were routed to the legacy path by `edgezero_can_handle_settings`. Now that `buffer_publisher_response_async` collects the dispatched auction, EdgeZero can run the full ad stack: - Pass the configured `creative_opportunities.slot` and finalize via `buffer_publisher_response_async` (the path's `ec.ec_context` already carries consent + platform geo). EID targeting stays off (`registry: None`). - Drop the `edgezero_can_handle_settings` gate, its routing branch, the three tests, and the now-unused test settings helpers — EdgeZero handles configured slots, so the legacy fallback for them is obsolete.
The EdgeZero publisher path dispatched the auction with registry: None, so the bid request carried no KV identity-graph EIDs (only client cookie EIDs). It already passes ec.kv_graph as the identity KV, so wire the matching PartnerRegistry::from_config(settings.ec.partners) into the AuctionDispatch to resolve server-side partner EIDs — matching the legacy auction path. Fastly-only: the sync EC identity graph (KvIdentityGraph/EcKvStore) works on Fastly's sync KV; the async-KV portability adapters are unaffected (they still pass registry: None until the EC graph supports async stores).
Resolve the fifth code-review pass. The blocking findings were all cross-adapter parity gaps in the server-side auction: - Build the geo-aware EC context in the /auction handlers on Axum, Cloudflare, and Spin. They passed EcContext::default(), leaving jurisdiction Unknown and failing the consent gate closed even for consented users. A shared per-adapter build_ec_context helper now serves /auction, page-bids, and the publisher fallback, and logs (rather than swallows) a malformed-consent read error. - Wire GET /__ts/page-bids and its OPTIONS->403 CSRF guard on the Fastly EdgeZero path and all three portability adapters, reusing core handle_page_bids and a shared page_bids_preflight_denied() helper. Previously it was Fastly-legacy-only, so SPA re-auction silently fell through to the origin on every other path. - Add trusted_server_core::response_privacy with the Set-Cookie cache-privacy downgrade and the uncacheable-operator-header guard, and call it from every adapter's apply_finalize_headers so a shared cache (Cloudflare) can no longer serve an operator/origin public Cache-Control on a cookie-bearing response. Also address the inline and non-blocking findings: warn on a dropped dispatched auction for bodiless responses, extract build_slot_json shared by the initial-page and page-bids paths, use creative_opportunity_slots() everywhere, drop the PBS id->ad_id fallback, log APS slot-id collisions, align the parallel provider parse with the collect path, remove the dead sync buffer_publisher_response, factor the mediator placeholder request, drop the unused toml dependency, guard MediaType against a future serde(default), and document the Fastly-only KV EID enrichment. JS: dedup win/billing beacons across concurrent renders, add the SSR guard to installSlimPrebidLoader, and short-circuit waitForSlotElements on an already-aborted signal. Add regression tests for the currentPath rollback and the u32::MAX format-dimension rejection.
Adopt main's ts-CLI config model over the branch's build-time embedding: drop the build.rs config generation and [build-dependencies], accept the trusted-server.toml deletion, remove the now-orphaned creative_slot_build_check module, and migrate the [creative_opportunities] example into trusted-server.example.toml. Slot validation is preserved via Settings::prepare_runtime, which the CLI runs at config push time. Adopt main's refactored EdgeZero finalize path in the Fastly adapter and centralize the per-user Set-Cookie cache-privacy guard in send_edgezero_response so it covers every EdgeZero send path. Fix three core test helpers for main's Body::into_bytes -> Option change.
Rebuilds feature/ts-cli-ad-templates fresh on server-side-ad-templates-impl (which already carries the ts CLI #799 + audit #800 via main), dropping the redundant ts-cli-base history that caused the merge conflicts. Adds the ad-template CLI: - ts audit ad-templates verify — browser/CDP ad-template slot verification - ts audit page — read-only page summary - ts config ad-templates — server-side ad-template diagnostics - shared CLI app-config loader + ad-template evidence/output models Keeps #800's URL->draft-config bootstrap by relocating it under audit/generate/ and exposing it as ts audit generate. Core: extracts the server-side ad-stack gate into creative_opportunities::evaluate_ad_stack_gate (three-state Yes/No/Unknown) so the CLI and the runtime publisher path share one gate definition.
prk-Jr
force-pushed
the
feature/ts-cli-ad-templates
branch
from
73df61a to
4ec0f3f
Compare
- Stop forwarding client-supplied X-Forwarded-For to Prebid Server; synthesize it from the platform-attested client IP instead - Make the APS slot ID remapping request-scoped by threading the AuctionRequest through parse_response_with_context, removing the shared provider-instance mutex that concurrent auctions could race - Guard dispatch_auction against multi-provider fan-out on platforms whose HTTP client executes requests sequentially, mirroring run_providers_parallel - Reject negative and non-finite creative-opportunity floor prices at config validation - Re-run the Set-Cookie cache-privacy downgrade after operator response headers are applied so configured Set-Cookie plus public cache headers cannot produce a shared-cacheable response - Install Prebid user ID modules immediately when the bundle loads after window.load (slim-Prebid lazy path), with a once-only listener - Drop allow-same-origin from debug ADM fallback iframes and validate extracted iframe src schemes to http(s) only
Reconstruct [creative_opportunities] slots from a live page's GPT registry and gampad/ads requests, and write them into an existing trusted-server.toml in place, preserving all other sections. Slots merge across runs: --page-pattern unions patterns into a re-seen slot, existing slots are preserved, and --replace wipes. Ephemeral div-id noise (React hashes, -container, hex UUIDs) is normalized to stable prefixes so verify matches across renders, and TOML keys/strings are escaped defensively. Add --cookie to ad-templates generate and verify so a valid bot-protection clearance cookie can carry the browser audit past an origin challenge. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reconcile the ad-template CLI + generate feature onto the commands/ module layout the base branch adopted from main (ts dev proxy, #798): - audit/ -> commands/audit/ : mod.rs is the audit namespace; the #800 draft-gen plus slot reconstruction move under commands/audit/generate/ (base's duplicate commands/audit/{analyzer,browser_collector} dropped). - config_ad_templates.rs -> commands/config/ad_templates.rs. - app_config and ad_templates support modules stay at crate root. - run.rs/lib.rs wiring merged with the new `ts dev` command. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Roll SPA page-bids navigation `currentPath` back to the last applied path instead of the immediately-previous one, so an aborted-then-failed navigation can no longer strand a route behind the no-op guard; add a regression test. - Add a concurrent render-bridge test: two same-adId messages before the cache fetch resolves must collapse to one fetch (in-flight gate), two beacons. - Assert `OPTIONS /__ts/page-bids` is denied with 403 on every adapter (Axum/Cloudflare/Spin) in cross-adapter parity. - Add a `u32::MAX` banner-format test covering the imp-drop branch when all formats exceed `i32::MAX`. - Dedup the page-bids GET 403 into `page_bids_preflight_denied()`. - Fix stale comments/docs: `buffer_publisher_response_async`, soften the oversized-body comment, and correct the `firedBeacons` key doc.
Resolve #744 (legacy entry-point removal + streaming publisher) against the server-side auction: keep the buffered auction path on all adapters, supersede the streaming publisher for publisher navigation. Verified: fastly/core/spin/ axum/cloudflare tests + clippy + fmt clean.
- Escape page-controlled slot fields and validate the gampad network id before splicing scraped values into trusted-server.toml - Add navigation and teardown timeouts to the verify browser collector - Snapshot evidence before the scroll pass so load-time entries keep phase initial_load; add a Chrome-gated regression fixture - Cap collector evidence lists in the injected script and after decode - Preserve CRLF line endings and render non-finite floor_price as valid TOML when updating configs in place - Cover all 128 gate combinations in the core ad-stack mirror test - Extract slot TOML rendering/merging/splicing into slot_toml.rs
…nto feature/ts-cli-ad-templates
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
Collaborator
There was a problem hiding this comment.
Automated review: I reviewed the ad-template CLI diagnostics/generate/verify changes against server-side-ad-templates-impl. I found three non-blocking issues around in-place TOML rewriting and generated-slot defaults that should be addressed, but no blocking correctness/security/data-loss issue severe enough to request changes. CI checks currently shown by GitHub are passing (integration tests, Fastly EC lifecycle, browser integration tests, prepare integration artifacts).
- Recognize [creative_opportunities] table headers carrying inline comments in the in-place splice and replace_key_in_section, so a valid operator config is updated instead of gaining a duplicate section - Default generated page_patterns from the recorded post-redirect final URL instead of the requested URL, falling back to the requested URL when the recorded final URL is invalid - Escape DEL (U+007F) in toml_string, which TOML basic strings reject alongside chars below U+0020 Each fix carries a parse-backed regression test.
Main squash-merged the same server-side ad-templates feature (#680), then added Tinybird auction telemetry (#818), edgezero v0.0.4 pins (#862), and external first-party Prebid bundle loading (#743). Resolutions: - Files where this branch matched the impl branch tip take main's version (telemetry/prebid-bundle additions on top of identical feature code): orchestrator, endpoints, openrtb, prebid integration, fastly app, .gitignore, prebid docs - creative_opportunities.rs keeps this branch's version (adds the shared ad-stack gate and pattern machinery used by the ts CLI; main's copy was identical minus those additions) - publisher.rs takes main's version with this branch's should_run_server_side_ad_stack delegation to evaluate_ad_stack_gate re-applied - run.rs keeps both sides' CLI tests (ad-templates/audit + prebid bundle)
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
Collaborator
There was a problem hiding this comment.
Automated review: I reviewed PR #823 against main, focusing on the new ts CLI ad-template diagnostics, browser audit/generate paths, TOML rewriting, and the shared runtime gate. CI is currently green. I did not find a blocking runtime/security issue requiring REQUEST_CHANGES, but I found several high-confidence non-blocking correctness and compatibility issues called out inline. Existing earlier automated comments about inline-commented TOML headers, redirected default page patterns, and DEL escaping appear to have been addressed in the current head, so I did not repeat them.
| match &args.command { | ||
| Some(AuditSubcommand::Page(page_args)) => page::run_page(page_args), | ||
| Some(AuditSubcommand::AdTemplates(AuditAdTemplatesCommand::Generate(gen_args))) => { | ||
| let loaded = crate::app_config::load_settings(&gen_args.config)?; |
Collaborator
There was a problem hiding this comment.
Automated review: P1 — ad-templates generate can persist env-overlay config back into the raw file.
load_settings applies the EdgeZero environment overlay by default, and this path passes the resulting effective creative_opportunities into the in-place merge while writing loaded.app_config_path. That means environment-provided slots / bidder params / network IDs can be copied into trusted-server.toml, or can replace/merge over the file's real slots, even though the operator is asking to rewrite the local config file. In Trusted Server deployments those overlays are often environment-specific; persisting them can corrupt the checked local config and may leak env-only ad parameters into a file.
Suggested fix: resolve the app-config path with the shared flags, but load the merge base for this mutating command with env_overlay = false (or reject unless --no-env is set). Keep env overlays for read-only diagnostics/verify where an effective view is expected. Add a regression test where an env overlay contributes a slot and generate --dry-run does not write that slot unless it exists in the raw file.
| report_error("target config has no [creative_opportunities] section to update") | ||
| })?; | ||
|
|
||
| let is_slot_table = |line: &str| { |
Collaborator
There was a problem hiding this comment.
Automated review: P1 — inline slot = [...] configs are left behind and make the rewritten TOML invalid.
The runtime config intentionally accepts slot via vec_from_seq_or_map, so a valid config can use slot = [{ ... }] (or map form) under [creative_opportunities]. The splice logic only treats [[creative_opportunities.slot]] and provider subtables as existing slots, so it inserts the generated [[creative_opportunities.slot]] blocks after the scalar block while leaving the old slot = ... key in place. TOML then rejects the result because creative_opportunities.slot was already defined as a value before being used as an array of tables.
Suggested fix: when splicing, detect a slot = ... key in the [creative_opportunities] scalar block and either remove/replace that whole value or fail with a clear message asking the operator to normalize to array-of-tables before generation. A TOML document/editing library would make this safer than line splicing. Add coverage for both inline array and map forms.
| generate::run_generate(generate_args, &collector, &mut out) | ||
| } | ||
| None => match &args.legacy_url { | ||
| Some(url) => page::run_page_url(url, false), |
Collaborator
There was a problem hiding this comment.
Automated review: P1 — the legacy ts audit <url> command now silently does a different operation.
On main, ts audit <url> bootstraps js-assets.toml and a draft trusted-server.toml by default, with flags such as --no-config, --js-assets, --config, and --force. This PR leaves ts audit <url> as a successful hidden alias, but routes it to the read-only page summary, so existing onboarding scripts can exit 0 while producing no config artifacts. The README still describes ts audit https://publisher.example as the bootstrap command, which makes the silent behavior change especially easy to miss.
Suggested fix: preserve the old artifact-generation behavior for the bare alias until a deprecation window expires, or make the bare alias fail with a migration message pointing to ts audit generate <url> instead of silently succeeding as page. Also update the public docs/README examples if the command shape intentionally changed.
| } | ||
|
|
||
| /// Derives a slot id from a div id by stripping the common GPT prefix. | ||
| fn slot_id_from_div(div_id: &str) -> String { |
Collaborator
There was a problem hiding this comment.
Automated review: P2 — generated slot IDs can fail Trusted Server's own runtime validation.
slot_id_from_div strips div-gpt-ad- and otherwise copies the page-controlled div ID directly into the config id. Runtime validation later requires slot IDs to contain only [A-Za-z0-9_-], while real GPT div IDs often contain characters like ., :, spaces, or other framework separators. In those cases generate writes TOML successfully, but ts config validate / runtime preparation rejects the generated config even though the raw div_id itself could have remained usable as a prefix.
Suggested fix: derive a separate safe slot id (sanitize invalid characters, collapse repeats, ensure non-empty, and make it unique), while preserving the original/normalized div stem in div_id. Prefer reusing validate_slot_id in the generator path and warning/skipping only if a safe ID cannot be produced.
| impl RenderSlot { | ||
| /// The stable identity used to match slots across runs: the div id (or slot | ||
| /// id), with any trailing `-` trimmed so hand-authored stems still match. | ||
| fn key(&self) -> String { |
Collaborator
There was a problem hiding this comment.
Automated review: P2 — rerunning generate does not merge configured div-prefix slots with their live divs.
The config model treats div_id as a prefix, and the PR docs even show div_id = "ad-atf-" matching live IDs such as ad-atf-0. This merge key only trims trailing hyphens and then requires equality, so a hand-reviewed prefix (ad-atf-) will not match the next discovered live div (ad-atf-0). A follow-up generate run appends a duplicate slot instead of unioning the new page pattern into the existing configured slot.
Suggested fix: when comparing a discovered slot to existing slots, treat an existing div_id as a prefix candidate (or reuse the verifier/runtime prefix matching rules) before falling back to the exact normalized key. Add a regression test with an existing prefix and a discovered suffixed div.
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
4 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
[creative_opportunities]) configuration: static path/slot diagnostics viats config ad-templates …, and browser-backed live verification viats audit …(local Chrome/Chromium over CDP).ts audit ad-templates generate <url>to bootstrap[creative_opportunities]slots from a live page's GPT state — reconstructed from the GPT registry (googletag.pubads().getSlots()) and capturedgampad/adsrequests — and write them into an existingtrusted-server.tomlin place, preserving every other section. Slots accumulate across pages (--page-patternunions coverage into re-seen slots), so multi-page pattern sets can be built run-by-run. A--cookieflag carries a valid session past an origin challenge (this tool still does not evade bot detection — it forwards a clearance cookie a human already earned).chromiumoxide) are excluded from thewasm32-wasip1build, and the runtime ad-stack gate is shared withpublisher.rsso the CLI cannot drift from server behavior.closes #701
Changes
trusted-server-core/src/creative_opportunities.rs[creative_opportunities]config types,match_slots, and a sharedevaluate_ad_stack_gateruntime gatetrusted-server-core/src/publisher.rsshould_run_server_side_ad_stackthrough the shared gate (behavior-preserving)trusted-server-cli/src/commands/config/ad_templates.rsts config ad-templates {lint,match,check,explain}static diagnosticstrusted-server-cli/src/app_config.rstrusted-server-cli/src/ad_templates/{expected,compare,output}.rstrusted-server-cli/src/commands/audit/{mod,page,collector,browser,ad_templates}.rs,commands/audit/ad_template_collector.jsts audit page+ts audit ad-templates verify: chromiumoxide collector, read-only GPT/APS/DOM init script, verifier orchestrationtrusted-server-cli/src/commands/audit/generate/{mod,gpt_slots}.rsts audit ad-templates generate: reconstruct slots from live GPT (registry +gampad/ads), normalize ephemeral div-id noise (React_R_hashes,-container, hex UUIDs) to stable prefixes, merge into the existing config in placetrusted-server-cli/src/commands/audit/generate/{browser_collector,collector,analyzer}.rsgenerate/; collector scrapes the live GPT registry and supports operator cookiestrusted-server-cli/src/run.rs,src/lib.rsauditnamespacetrusted-server-cli/Cargo.tomledgezero-core+serde_jsondeps (cfg-gated off wasm, like the existing browser deps)docs/superpowers/{specs,plans}/2026-06-26-server-side-ad-template-cli*Closes
Not yet linked to an issue.
Test plan
cargo test --workspacecargo clippy --workspace --all-targets --all-features -- -D warningscargo fmt --all -- --checkcd crates/trusted-server-js/lib && npx vitest run(393 passed)cd crates/trusted-server-js/lib && npm run formatcd docs && npm run formatcargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1(pluscargo build -p trusted-server-cli --target wasm32-wasip1— browser deps stay out of wasm)cargo test -p trusted-server-cli --target <host-triple>(164, incl. slot reconstruction, div-normalization, merge/union, TOML-escaping/CRLF tests + Chrome-gated browser fixtures for evidence collection and scroll-phase attribution); manualts audit ad-templates verifyagainst a local fixture —confirmed(exit 0) and--stricton a missing slot (exit 2)How to use
Configure slots
In your (gitignored)
trusted-server.toml— fictional values shown:Generate slots from a live page (needs local Chrome/Chromium)
Static diagnostics (no browser)
Browser-backed audit (needs local Chrome/Chromium)
Shared config flags (all of the above)
Exit behavior
verifyis auditor-assist: exits0even with missing/partial evidence.--strictexits non-zero when a matched slot is missing or only partially confirmed (CI gate). A page-level navigation failure also exits non-zero.[auction].enabled = false) mark a page "skipped" so--strictdoes not fail it.Local live test (deterministic, no external site)
Many large ad publishers block headless/non-evasive browsers, so
verifyagainst them sees a challenge page, not the article (this tool does not evade bot detection). To exercise the full pipeline reliably, serve a local fixture:Checklist
unwrap()in production code — useexpect("should ...")println!/eprintln!in library code (CLI output useswriteln!; errors uselog)