fix(store): survive iframe/tag load races for direct-iframe embeds by Redskull-127 · Pull Request #70 · trysurface/scripts

Redskull-127 · 2026-07-21T20:12:13Z

Problem

When a Surface form is embedded as a direct iframe (customer pastes the iframe, tag loads separately), the handshake was one-shot and answer-only:

The tag deferred its message listener until DOMContentLoaded and never pushed data proactively, so a fast-booting iframe's SEND_DATA was silently dropped.
Result: no attribution (UTMs/referrer/cookies), no journey stitching, duplicate-lead risk, no partial-fill — all degrading invisibly while the form still renders and submits.

Changes

src/store/message-listener.ts — attach the window message listener immediately. Nothing in the handler needs the DOM (iframes are queried at message time); the DOMContentLoaded deferral was pure message loss.
src/store/store.ts — push STORE_UPDATE once on store init (at DOM-ready when loaded in <head>, immediately otherwise), plus LEAD_DATA_UPDATE only when cached lead data exists. Identify stays SEND_DATA-driven, so the tag still never creates a lead on pages where no form asked.
test/direct-iframe.html (new) — reproduces the race: hard-coded iframe with a delayed, cache-busted tag loader (?tagDelay=<ms>, ?noTag=1), SPA route-change button for the clobber scenario. The existing test pages create the iframe via the tag, so they can't reproduce this.

Verification (browser, real deployed forms)

Race reproduced pre-fix mechanics: form's SEND_DATA at ~3s unanswered when the tag was injected at 4.5s.
Rescue verified: the init push delivered the payload to the already-running form; lead identity converged to the existing lead (single deduped identify, no duplicate).
Clobber check: typed input survived a host pushState → route-change STORE_UPDATE.
pnpm run typecheck + pnpm run build clean; bundle artifacts rebuilt.

Pairs with trysurface/surface_forms#4959 (forms-side: 500ms SEND_DATA polling + wider no-tag fallback + prefill guard). Each side is independently safe against old deployed versions of the other; deploy the forms side first (instant), tag second (CDN-cached).

🤖 Generated with Claude Code

Direct-iframe embeds lost the entire store payload (attribution, lead identity, journey id, partial fill) whenever the form iframe booted before the tag was listening: the tag deferred its message listener to DOMContentLoaded and never pushed proactively, so the form's one-shot SEND_DATA went unanswered. - attach the message listener immediately; nothing in the handler needs the DOM and the deferral silently dropped early SEND_DATA - push STORE_UPDATE once on store init (plus LEAD_DATA_UPDATE when lead data is cached) so iframes that asked before the tag loaded still get their payload; identify remains SEND_DATA-driven - add test/direct-iframe.html to reproduce the race with ?tagDelay=<ms> and ?noTag=1 (customer-embedded iframe, no tag involvement) Pairs with the forms-side SEND_DATA retry change; each side is safe against old deployed versions of the other. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

greptile-apps · 2026-07-21T20:17:52Z

Greptile Summary

This PR fixes a silent data-loss race in direct-iframe embed scenarios: when a customer hard-codes the Surface iframe and the tag loads separately, a fast-booting form's SEND_DATA could be sent before the tag's message listener was attached, dropping attribution, journey stitching, and prefill data entirely.

message-listener.ts: Removes the DOMContentLoaded deferral so the window message listener is attached immediately on module init — nothing in the handler requires the DOM, so the deferral was pure message loss.
store.ts: Adds a proactive init push (STORE_UPDATE + optional identify + LEAD_DATA_UPDATE) triggered on store construction whenever a Surface iframe is already present in the DOM; the push is deferred to DOMContentLoaded when the document is still loading, or to setTimeout(0) otherwise. Also adds a journey-id re-push on async resolution in both the init and route-change setters.
Tests and CI: New unit tests cover the key regression (listener must fire during readyState=loading), the boot-push scenarios, and journey re-push; a new CI workflow enforces typecheck, unit tests, and bundle freshness.

Confidence Score: 5/5

Safe to merge. The fix is narrowly targeted: the listener timing change is a one-liner with a clear safety argument (handler never touches the DOM), and the init push is gated behind an iframe presence check so pages without Surface forms are unaffected.

The core logic is correct and well-reasoned. The only finding is a minor test coverage gap — the DOMContentLoaded scenario verifies the synchronous STORE_UPDATE push but doesn't await microtask resolution to confirm LEAD_DATA_UPDATE follows — which doesn't affect production behavior and is already covered by the parallel setTimeout-path test.

store.test.ts — the DOMContentLoaded scenario could add a microtask flush to verify the full push sequence matches the setTimeout-path test.

_{Reviews (5): Last reviewed commit: "test: add regression suite + GitHub CI f..." | Re-trigger Greptile}

Redskull-127 · 2026-07-21T20:19:38Z

@greptile please check, embedding Surface Tag in head with no defer or async - will not cause any issue?

greptile-apps · 2026-07-21T20:20:27Z

No issues from a correctness standpoint. Here's why it's safe:

Message listener — window.addEventListener("message", handleMessage) now runs immediately. window is always available in <head> scripts, so this attaches cleanly.

pushInitialData — the code explicitly guards against the not-yet-ready DOM:

if (document.readyState === "loading") {
  document.addEventListener("DOMContentLoaded", pushInitialData);
} else {
  pushInitialData();
}

When loaded in <head> without defer/async, document.readyState will be "loading", so pushInitialData (which calls sendPayloadToIframes and queries for iframes) is correctly deferred until DOM is ready. The PR description even calls this out explicitly: "push STORE_UPDATE once on store init (at DOM-ready when loaded in <head>, immediately otherwise)".

The one thing to be aware of is render-blocking behavior — a plain <script> in <head> blocks HTML parsing until the script finishes. This is a pre-existing concern not introduced by this PR, and for a small tag script it's typically acceptable. But if customers are sensitive to page load performance, defer or async would be preferable.

The browser caches ../surface_tag.js across rebuilds, so delay scenarios could run a stale bundle. Append a timestamp query so every injection fetches the current build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

AminHakem

Two blocking race-handling gaps remain. Inline comments include the reproduction evidence and suggested direction.

…covery The init push only replayed synchronous state: on a first visit there is no cached lead and the journey id is still resolving, so a one-shot form whose SEND_DATA was missed received STORE_UPDATE with null lead/journey data and nothing ever followed. - a Surface iframe already in the DOM now implies a missed request: the push runs identifyLead and follows with LEAD_DATA_UPDATE, same as an answered SEND_DATA; pages without a Surface iframe stay quiet - the journey-id setter re-pushes STORE_UPDATE when the id resolves so already-notified iframes get the pageview stitched - the immediate-push branch defers one task so index.ts exposes the store first; direct-iframe.html installs a setter trap and hooks notifyIframe on assignment, making the init push observable in the event monitor Verified with cleared storage + ?tagDelay=8000 against the deployed one-shot form: STORE_UPDATE (null journey) -> STORE_UPDATE (resolved journey) -> LEAD_DATA_UPDATE (lead + journey), all logged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Same staleness the init path had: the route-change handler pushes STORE_UPDATE immediately, but a journey created or refreshed by that route change resolves after the push. Re-push from the setter when the id actually changes, mirroring the constructor path. Found by multi-agent review of the branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Redskull-127 · 2026-07-21T21:16:50Z

Follow-up from a multi-agent adversarial review over both branches: one additional fix landed in 40e05e3 — the SPA route-change handler pushed STORE_UPDATE before a journey created/refreshed by that navigation resolved, so iframes kept a stale journey id until the next push. The journey setter now re-pushes when the id actually changes, mirroring the init path added in 18dd6a3.

The review also confirmed the two issues Amin raised (both fixed in 18dd6a3) and refuted several suspected regressions, including "pushInitialData should be gated off on Surface-owned origins" and "the LEAD_DATA_UPDATE cache double-read can send a null payload".

Vitest+jsdom unit tests locking in the postMessage-race fixes on this branch: immediate message-listener attach, iframe-gated boot push (no push/identify without a Surface iframe), cached-lead path, journey-id re-push on init and route change, and the sender/domain protocol shape. CI runs typecheck, the test suite, and a bundle-freshness check (rebuild + git diff) on every PR to main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Redskull-127 requested review from AminHakem and arjunsahai July 21, 2026 20:20

test(direct-iframe): cache-bust the delayed tag loader

de11888

The browser caches ../surface_tag.js across rebuilds, so delay scenarios could run a stale bundle. Append a timestamp query so every injection fetches the current build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

AminHakem reviewed Jul 21, 2026

View reviewed changes

Comment thread src/store/store.ts

Comment thread test/direct-iframe.html Outdated

Redskull-127 and others added 2 commits July 22, 2026 02:23

Redskull-127 requested a review from AminHakem July 21, 2026 21:36

AminHakem approved these changes Jul 21, 2026

View reviewed changes

Redskull-127 merged commit 31f25eb into main Jul 22, 2026
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(store): survive iframe/tag load races for direct-iframe embeds - #70

fix(store): survive iframe/tag load races for direct-iframe embeds#70
Redskull-127 merged 5 commits into
mainfrom
fix/postmessage-race-direct-iframe

Redskull-127 commented Jul 21, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Jul 21, 2026 •

edited

Loading

Greptile Summary

Confidence Score: 5/5

Uh oh!

Redskull-127 commented Jul 21, 2026

Uh oh!

greptile-apps Bot commented Jul 21, 2026

Uh oh!

AminHakem left a comment

Uh oh!

Uh oh!

Uh oh!

Redskull-127 commented Jul 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Redskull-127 commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Changes

Verification (browser, real deployed forms)

Uh oh!

greptile-apps Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Uh oh!

Redskull-127 commented Jul 21, 2026

Uh oh!

greptile-apps Bot commented Jul 21, 2026

Uh oh!

AminHakem left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Redskull-127 commented Jul 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Redskull-127 commented Jul 21, 2026 •

edited

Loading

greptile-apps Bot commented Jul 21, 2026 •

edited

Loading