fix(hubspot-web): add id=hs-script-loader to prevent duplicate script injection

[harsh-joshi99]

Summary

Adds { id: 'hs-script-loader' } to the HubSpot tracking-script loadScript call in the hubspot-web browser destination. HubSpot's own embed uses the hs-script-loader id to detect whether its script is already present; without it, the script can be injected twice.

Fixes STRATCONN-6721.

Root cause

HubSpot's analytics script (loaded transitively by js.hs-scripts.com/<portalId>.js) injects its region-specific tracking script through a guard keyed on the DOM id hs-script-loader. Confirmed verbatim in HubSpot's own code at js.hs-analytics.net/analytics/<ts>/<portalId>.js:

// invoked as: embedHubSpotScript('https://js-na1.hs-scripts.com/<portalId>.js', 'hs-script-loader')
embedHubSpotScript = function (t, e) {
  if (!( ...|| document.getElementById(e)) && this.trackingEnabled) {
    var n = document.createElement("script");
    n.src = t; n.type = "text/javascript"; n.id = e; // e === "hs-script-loader"
    // ...insertBefore
  }
};

Segment was injecting js.hs-scripts.com without that id, so document.getElementById('hs-script-loader') returned null, the guard didn't trip, and HubSpot injected a second (js-na1.hs-scripts.com) script. Setting the id on Segment's injected script makes the guard find it and skip the duplicate.

References:

• HubSpot bootstrap loader: js.hs-scripts.com/<portalId>.js (~1.9 KB; loads the analytics script below)
• HubSpot analytics script containing the embedHubSpotScript guard: js.hs-analytics.net/analytics/<ts>/<portalId>.js
• Jira: STRATCONN-6721
• HubSpot docs — Install the HubSpot tracking code (does not show the snippet itself; the guard lives in the served scripts above)

Changes

• src/index.ts: pass { id: 'hs-script-loader' } to deps.loadScript(scriptPath, ...)
• src/__tests__/index.test.ts: add a test asserting the injected HubSpot script carries id="hs-script-loader"

Testing

Verified locally via the Action Tester with a real HubSpot portal ID, comparing main vs this branch.

	Scripts injected
Before (main)	2 — js.hs-scripts.com (id:'') + duplicate js-na1.hs-scripts.com (id:'hs-script-loader')
After (this branch)	1 — js.hs-scripts.com (id:'hs-script-loader'), no duplicate

Before — duplicate script injected (main, unfixed):

After — single script with id=hs-script-loader (this branch):

• New unit test added for the script id.
• ⚠️ Note: the hubspot-web jest suite currently fails to construct its jsdom environment locally (TypeError: Cannot read properties of undefined (reading 'html') in JSDOMEnvironment) — a pre-existing tooling mismatch (jest 30 vs ts-jest <28), unrelated to this change. CI will run the canonical suite.

Deploy note

These changes have also been pushed directly to staging for testing.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR updates the HubSpot Web (Actions) browser destination to align with HubSpot’s own loader-script duplication detection by ensuring the injected tracking script uses the hs-script-loader DOM id, and adds a unit test for that behavior.

Changes:

• Passes { id: 'hs-script-loader' } when loading the HubSpot tracking script in destination initialization.
• Adds a Jest test to assert the HubSpot script tag carries id="hs-script-loader".

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
packages/browser-destinations/destinations/hubspot-web/src/index.ts	Adds id: 'hs-script-loader' to the HubSpot loader script injection call.
packages/browser-destinations/destinations/hubspot-web/src/tests/index.test.ts	Adds a unit test asserting the injected HubSpot script has the expected id.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[harsh-joshi99]

Note

Self-Review
Posted by PR author (harsh-joshi99) — self-reviews cannot apply the APPROVED label. Intended verdict: Manual Review Needed ⚠️. Ask a teammate to run plugin-review for the APPROVED label.

Code Review Personae Verdict: Manual Review Needed ⚠️

The one-line change (adding id='hs-script-loader' to the injected HubSpot tracking script) introduces no crashes, NPEs, resource leaks, races, or security issues, and the new test is correct and not flaky. All 3 bug-finder passes independently converged on a single design concern: loadScript dedups by src/status, not by id, so the fix is asymmetric and can produce a duplicate DOM id in multi-instance edge cases.

Policy scan clean.

---

📝 Deep Code Review (3 Passes) — 0 Critical, 0 High, 1 Medium, 2 Low 🟡

Consensus: all 3 independent passes flagged the same line (src/index.ts:95) and the same underlying mechanism — the runtime loadScript() dedups by s.src === src (load-script.ts:3) and short-circuits only when an existing matching script carries a Segment status attribute of 'loaded'/'loading'. The new id is never used for de-duplication. The single-instance / same-region path that STRATCONN-6721 targets is fully fixed (HubSpot's own embed guard does document.getElementById('hs-script-loader') and now finds Segment's script). The residual concern is asymmetry: (a) if a customer already has HubSpot's native embed snippet on the page (same src, no Segment status attr), Segment still falls through and injects a second <script id='hs-script-loader'>; (b) two HubSpot Web instances with differing src (EU + non-EU, or differing portalId) each inject an element with the same hardcoded id, producing a duplicate DOM id and a potentially wrong getElementById target. Both are narrow edge cases, not production crashes. The new test's toHaveLength(1) is sound — setup-after-env.ts resets document.head per test and the harness's empty <script> has no id, so no cross-test leakage or false match.

Findings

• 🟡 MEDIUM: Fix is asymmetric: loadScript dedups by src, not id (packages/browser-destinations/destinations/hubspot-web/src/index.ts:95)
  [Found by 3/3 passes] HubSpot's embed guard skips injection when document.getElementById('hs-script-loader') exists, which this PR now satisfies — good. But the runtime's own loadScript() (browser-destination-runtime/src/load-script.ts:3) dedups only by s.src === src and returns early only when the found script has a Segment-set status attribute of 'loaded'/'loading'. Two consequences: (1) If the customer already has HubSpot's native embed snippet on the page (same src https://js.hs-scripts.com/{portalId}.js, id present but no Segment status attr), loadScript falls past the dedup branch and injects a SECOND <script id='hs-script-loader'> — the opposite direction of the bug being fixed. (2) Two HubSpot Web instances whose src differs (EU js-eu1.hs-scripts.com + non-EU js.hs-scripts.com, or two portalIds) each create an element carrying the same hardcoded id, yielding a duplicate DOM id and a getElementById target that resolves to whichever is first in document order. Neither is a crash; the common single-instance/same-region case (the STRATCONN-6721 target) is fully fixed.
  Suggestion: Optionally mirror HubSpot's own guard before injecting — e.g. if (document.getElementById('hs-script-loader')) return window._hsq in initialize() — or have loadScript dedup by matching id when one is supplied. For multi-instance support, derive a per-instance id (include portalId/region) instead of a fixed string. If multi-portal-per-page is out of scope for this ticket, no change needed.
• 🔵 LOW: Test asserts resolved IDL src; would break if URL became relative (packages/browser-destinations/destinations/hubspot-web/src/__tests__/index.test.ts:50)
  loaderScripts[0].src returns the IDL-resolved absolute URL. Because scriptPath is already absolute, resolution is an identity and the assertion holds correctly today. Noted only because it would silently break if the injected URL ever became relative.
  Suggestion: Optional: assert via getAttribute('src') to test the literal injected value rather than the resolved IDL value.
• 🔵 LOW: Pre-existing: tag.parentElement throws when document has zero <script> tags (packages/browser-destination-runtime/src/load-script.ts:45)
  getElementsByTagName('script')[0] is undefined if the page has no script tags, so tag.parentElement?.insertBefore would throw 'Cannot read properties of undefined'. NOT introduced or newly exercised by this PR (the id attribute changes nothing on this branch), and the test harness avoids it by appending an empty <script> in beforeEach. Flagged for completeness only; out of scope for this PR.
  Suggestion: Out of scope. If hardened later, fall back to document.head when no script tag exists.

🛡️ Policy Compliance Review — No violations (3 policies evaluated)

Evaluated 3 policies across all review surfaces. No violations detected.

Review Stages Executed

• ✅ Deep Code Review (3 Passes)
• ✅ Policy Compliance Review
  _{Generated by code-review-personae v0.8.7 | Deep Code Review (3 Passes), Policy Compliance Review}

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.