fix(worker): make the Google Ads conversion tag env-driven instead of a dead placeholder - #52
fix(worker): make the Google Ads conversion tag env-driven instead of a dead placeholder#52nish3451 wants to merge 11 commits into
Conversation
… a dead placeholder The funnel's only Google Ads conversion measurement was dead by construction: brief-requested.html hardcoded the gtag loader with a placeholder conversion id, brief-requested.js fired the event to the same placeholder, and the production CSP blocked googletagmanager.com entirely — so even a real id pasted in would never load or record. The tag is now generated by the worker at request time from GOOGLE_ADS_CONVERSION_ID / GOOGLE_ADS_CONVERSION_LABEL, emitted only on /brief-requested when both are configured and well-formed, with gtag's CSP allowances scoped to that one noindex page's response. With either value missing or malformed, the page ships with no tag at all. CI now refuses any placeholder or hardcoded gtag in public/ or src/worker.js, so the dead-by-construction shape cannot return; the tracking spec documents the secret puts that activate the tag once the console-side conversion action exists.
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Warning Review limit reached
Next review available in: 59 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughGoogle Ads conversion tracking now uses validated Worker environment values. The Worker conditionally injects tracking and scoped CSP rules for ChangesGoogle Ads conversion tracking
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Request
participant Worker
participant Environment
participant Browser
Request->>Worker: Request /brief-requested
Worker->>Environment: Read conversion ID and label
Environment-->>Worker: Return configuration
Worker->>Browser: Serve rewritten HTML and scoped CSP
Browser->>Worker: Request generated conversion script
Worker-->>Browser: Return gtag loader and conversion event
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33eb558ce5
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // (see googleAdsConversion below); when they are not, the page ships with | ||
| // the strict CSP and no tag at all. | ||
| const GOOGLE_ADS_CSP = | ||
| "default-src 'self'; img-src 'self' data: https://www.googleadservices.com; style-src 'self' https://fonts.googleapis.com; font-src 'self' https://fonts.gstatic.com; script-src 'self' https://static.cloudflareinsights.com https://www.googletagmanager.com; connect-src 'self' https://cloudflareinsights.com https://www.googletagmanager.com https://googleads.g.doubleclick.net https://www.googleadservices.com https://www.google-analytics.com https://stats.g.doubleclick.net; base-uri 'self'; frame-ancestors 'none'; form-action 'self'"; |
There was a problem hiding this comment.
Allow Google Ads pixels under img-src
When gtag sends a conversion or view-through beacon as an image request to googleads.g.doubleclick.net (and, for some Google Ads flows, www.google.com), this policy blocks it: placing DoubleClick only in connect-src does not authorize image loads, while img-src permits only www.googleadservices.com. This can silently discard conversion signals even though the tag loads successfully, corrupting the paid-search experiment; add every Google Ads image-beacon origin to img-src and exercise the configured Worker response in the browser check.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60dda491da
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const assetResponse = await env.ASSETS.fetch(request); | ||
| if (assetResponse.ok) { |
There was a problem hiding this comment.
Handle conditional 304s before injecting the tag
When a browser revalidates a previously cached, unconfigured /brief-requested page after the Ads secrets are enabled, the preserved static-asset ETag can make this fetch return 304. Because a 304 has ok === false, execution falls through and repeats the same conditional asset request, returning a strict-CSP 304 and causing the browser to reuse the cached tag-free HTML. Returning visitors—and potentially the post-activation verification browser—therefore remain untracked until their cache is bypassed; fetch the source HTML without request validators or prevent the unconfigured confirmation response from being cached.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d9e326287
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return withSecurityHeaders( | ||
| new Response(googleAdsScript(ads), { | ||
| headers: { "Content-Type": "text/javascript;charset=UTF-8" } | ||
| }) |
There was a problem hiding this comment.
Mark the generated conversion script non-cacheable
When the Google Ads ID or label is rotated after a visitor has loaded this endpoint, this generated 200 response has neither Cache-Control nor a config-specific URL, so a browser cache may reuse JavaScript containing the previous send_to value. The newly injected HTML can then load gtag for the new ID while the unchanged /brief-requested.js attributes the conversion to the old action; return this env-derived script with Cache-Control: no-store (and apply equivalent cache protection to the generated HTML).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29919a5e63
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| url.pathname === "/brief-requested" || url.pathname === "/brief-requested.html"; | ||
| const isBriefRequestedScript = url.pathname === "/brief-requested.js"; | ||
|
|
||
| if (ads && request.method === "GET" && (isBriefRequestedPage || isBriefRequestedScript)) { |
There was a problem hiding this comment.
Load the Google tag before the confirmation page
When a paid-search visitor lands on / or /audit with a gclid/wbraid, this condition excludes that landing response, so Google's conversion linker never gets a chance to persist the click identifier; htmlRedirect() then clears the query before navigating to /brief-requested. In browsers where third-party attribution is unavailable, the resulting event cannot be tied back to the campaign, causing the paid-search experiment to undercount conversions. Load the base Google tag/conversion linker on the landing pages while keeping the conversion event restricted to the confirmation page, or otherwise preserve the click identifier through the flow.
Useful? React with 👍 / 👎.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Refreshed onto current main and re-verified (lane-1 run, 2026-08-11). Branch merged latest
Ready to merge. Remaining step (console-side, per |
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Superseded by #136: this branch went stale and conflicts with current main (favicon commits touched the same files). The identical verified fix is re-applied onto fresh main in #136, with the same tests green (npm test 95/0, render-blocking PASS, deploy dry-run OK). Reopen if you'd rather merge this branch after a refresh. |
What
The funnel's only Google Ads conversion measurement was dead by construction:
public/brief-requested.htmlhardcoded the gtag loader with theAW-XXXXXXXXXplaceholder — a tag that can never record a conversion.public/brief-requested.jsfired the conversion event to the same placeholder (AW-XXXXXXXXX/YYYYYYYYYYYYYYYYYYY).script-src 'self' https://static.cloudflareinsights.com) blocksgoogletagmanager.comscripts and gtag's beacon endpoints, so the tag could never load or send under the served CSP.Fix
The tag is now generated by the Worker at request time from env values and only ever emitted when it can actually work:
src/worker.jsreadsGOOGLE_ADS_CONVERSION_ID+GOOGLE_ADS_CONVERSION_LABEL, validates both with strict patterns (AW-+ digits; 10+ char alphanumeric label), and only then:/brief-requestedHTML response, and/brief-requested.jswith the real config + conversion event.googletagmanager.comscript;googleads.g.doubleclick.net/googleadservices.com/google-analytics.com/stats.g.doubleclick.netconnect;googleadservices.comimg) are applied only to the/brief-requestedresponse via a page-scoped CSP — every other page keeps the strict CSP. With either value missing or malformed, the page ships with no tag at all rather than a dead one.public/files (brief-requested.jsis a documented no-op until configured).scripts/check-site.mjsrefuse: any placeholder inpublic/orsrc/worker.js, any hardcoded gtag in the served HTML, any static conversion firing inbrief-requested.js, and any drift of the worker's env wiring.specs/003-wellness-clinic-launch/tracking-setup.mdupdated: activation is nowwrangler secret put GOOGLE_ADS_CONVERSION_ID/GOOGLE_ADS_CONVERSION_LABEL(.dev.varsfor dev) instead of pasting into HTML.Activation (console-side, cannot be done in code)
Creating the "Brief requested" conversion action still requires the Google Ads console (the spec documents this as account-side work): create the action, copy the
AW-…ID and label,wrangler secret putboth, deploy, then verify with a live test submission before any spend.Validation
npm run check— passed (incl. new placeholder/guard checks)npm test— 85 tests pass, incl. 3 new: no tag while unconfigured; correct injection + page-scoped CSP when configured; nothing emitted for partial/malformed confignpm run check:render-blocking(real Chromium, production CSP) — all 6 pages PASS;/brief-requestedno longer triggers the gtag placeholder console errornpx wrangler deploy --dry-run— cleanSummary by CodeRabbit
New Features
Bug Fixes
Documentation