Skip to content

feat: add Chatwoot support widget to docs, stacked above back-to-top button - #910

Open
amaan-bhati wants to merge 5 commits into
mainfrom
ft/chatwoot-int
Open

feat: add Chatwoot support widget to docs, stacked above back-to-top button#910
amaan-bhati wants to merge 5 commits into
mainfrom
ft/chatwoot-int

Conversation

@amaan-bhati

@amaan-bhati amaan-bhati commented Sep 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

What has changed?

Adds the self-hosted Chatwoot support widget to the docs site, matching the landing page and the enterprise app, and fixes the bottom-right corner collision between the chat bubble and the back-to-top button.

  • static/scripts/chatwoot.js (new): sets window.chatwootSettings, boots the Chatwoot SDK, calls chatwootSDK.run() with the shared website token, and flags .chatwoot-ready on <html> once the widget is live.
  • src/metaPixelRouteTracker.js: splits the lazy loader into two engagement tiers so Chatwoot can use a narrower trigger than the analytics tags.
  • src/css/custom.css: sizes and positions .theme-back-to-top-button to match the bubble, scoped to .chatwoot-ready.
  • vercel.json: allows chatwoot.keploy.io in script-src, connect-src (plus wss:) and frame-src.
  • docusaurus.config.js: comment only, documents the new loading strategy.

Screenshot 2026-09-21 at 6 00 45 PM

The collision, and the numbers

  • Chatwoot's bubble is position: fixed, 64px square, 20px inset from the bottom-right, at z-index: 2147483000.
  • Docusaurus' back-to-top button is a 48px square fixed to the same corner at 1.3rem (20.8px), so the bubble covers it and swallows its clicks.
  • The button is less transient than it looks: per useBackToTopButton in @docusaurus/theme-common, it appears on any upward scroll past 300px and only hides when the reader scrolls back down, drops below the threshold, or clicks an anchor. No timeout, so the two share the corner for most of a page visit.
  • width/height: 64px matches the bubble. Once they are equal widths, the bubble's own right: 20px puts both centrelines 52px from the edge. At the theme's defaults the button sat 7.2px off that line.
  • bottom: 104px = 20px inset + 64px bubble + 20px gap, the same clearance the enterprise app uses for its toasts.
  • The chevron is a fixed 2rem mask on ::after, so growing the button would have dropped the glyph from 67% to 50% of it. mask-size scales it back to 2.5rem.
  • All of it is scoped to .chatwoot-ready, set from the SDK's ready event. If Chatwoot is down, blocked, or the SDK 404s, none of it applies and the button keeps the theme's own geometry rather than floating above an empty gap.

Screenshot 2026-09-21 at 6 06 09 PM

Loading strategy

src/metaPixelRouteTracker.js now holds two tiers instead of one list:

Tier Trigger Scripts
Analytics pointerdown / keydown / touchstart / scroll Clarity, Apollo
Support pointerdown / keydown / touchstart Chatwoot
  • Clarity genuinely wants scroll, since recording scroll is its job.
  • Chatwoot does not. It opens a persistent websocket, and on a docs site practically every session scrolls, so sharing the analytics trigger would boot the SDK for effectively every reader and undo the point of gating it.
  • The narrower gate is also exactly the one the landing page uses for the same widget, so docs and landing now behave identically.

Notes for reviewers (deliberate, not oversights)

  • No interaction gate inside chatwoot.js. The loader already gates it; a second gate would mean two competing mechanisms. The file does guard against a double run(), checked inside onload rather than at parse time, since at parse time $chatwoot is undefined for every injection and the check would never fire.
  • The website token is committed in plaintext, and that is fine. A Chatwoot websiteToken is a public client-side identifier, like a GA measurement ID, not a secret. It already ships in the landing page's JS bundle and is readable via view-source. No env var needed.
  • No !important on the new rules. .chatwoot-ready .theme-back-to-top-button is two classes, so it beats the theme's single-class CSS module on specificity regardless of bundle order. The pre-existing background-color declarations keep theirs.
  • Hardcoded px rather than rem. Every value is derived from the bubble's own fixed px geometry, which does not scale with root font size.
  • Optional chaining in a static/ file. Not transpiled by webpack, but static/js/apollo-init.js already uses the same syntax.
  • darkMode: "auto" follows the OS, not the Docusaurus toggle. Chatwoot exposes no hook for the latter. A reader on a light OS who flips docs to dark still gets a light widget. Better than always-light, not perfect.
  • No HubSpot removal. HubSpot chat is already absent from this branch, so there is no competing widget to disable.
  • No dependency changes.

This PR Resolves #(issue)

Type of change

  • New feature (non-breaking change which adds functionality).

How Has This Been Tested?

npm run build passes (exit 0), npx prettier --check clean on all five files. Verified in the emitted output:

  • chatwoot.js ships to build/scripts/, and its path is referenced from the emitted client bundle.
  • .theme-back-to-top-button{background-color:#ff914d!important}
  • .chatwoot-ready .theme-back-to-top-button{bottom:104px;height:64px;right:20px;width:64px}
  • .chatwoot-ready .theme-back-to-top-button:after{-webkit-mask-size:2.5rem 2.5rem;mask-size:2.5rem 2.5rem}
  • CSP carries chatwoot.keploy.io in script-src (loads /packs/js/sdk.js), connect-src plus wss: (ActionCable socket) and frame-src (widget panel iframe).
  • style-src and font-src are deliberately left alone. The SDK injects styles via createElement("style"), which 'unsafe-inline' already covers, ships no <link rel=stylesheet> and no @font-face, and the panel iframe is governed by the CSP on its own response rather than this one. Verified against the shipped bundle.

Verified via npm run serve:

  • Bubble appears after first interaction, exactly one bubble on the page
  • Websocket to wss://chatwoot.keploy.io connects
  • Scroll down then up: bubble and back-to-top button both visible and independently clickable
  • Mobile widths, since that corner is the thumb zone

Screenshot 2026-09-21 at 6 42 12 PM

Checklist:

  • My code follows the style guidelines of this project.
  • I have performed a self-review of my own code.

@amaan-bhati
feat: add Chatwoot support widget to docs, stacked above back-to-top …
1802c11 
…button

Signed-off-by: amaan-bhati <amaanbhati49@gmail.com>
Copilot AI lite review requested due to automatic review settings September 21, 2026 12:37
Copilot AI reviewed Sep 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@amaan-bhati
perf: align scoll btn and increase size wrt chat btn
ce2fe81 
Signed-off-by: amaan-bhati <amaanbhati49@gmail.com>
@amaan-bhati

amaan-bhati commented Sep 21, 2026

Copy link
Copy Markdown
Member Author

Code Review Agent, Iteration 1

Scope: 3 files, +51 / -3
CI: run-lint pass, prettier pass, DCO pass, asset-budget pass, deploy-preview pending

VERDICT: REQUEST CHANGES

1 blocking, 3 important, 5 minor.

BLOCKING

1. The repo ships a CSP, and Chatwoot is not in it

  • PR body claims: "No CSP changes. This repo ships no Content-Security-Policy headers (nothing in netlify.toml, no static/_headers)."
  • Wrong location. The CSP is in vercel.json:14, added by fd27e0f5, the commit right before this branch.
  • Nothing breaks today because it is Content-Security-Policy-Report-Only. It breaks silently the day anyone flips it to enforcing.
  • Missing allowances:
    • script-src needs https://chatwoot.keploy.io (loads /packs/js/sdk.js)
    • connect-src needs https://chatwoot.keploy.io wss://chatwoot.keploy.io (ActionCable socket)
    • frame-src needs https://chatwoot.keploy.io (the widget panel is an iframe)
    • img-src https: and style-src 'unsafe-inline' already cover it
  • Fix it in this PR, one line, alongside the widget it belongs to.

IMPORTANT

2. Back-to-top offset is unconditional

  • src/css/custom.css:1067 pushes the button to bottom: 104px always.
  • If Chatwoot is down, blocked, or the SDK 404s, the button floats with an empty 84px gap under it.
  • No onerror on the script either, so there is no signal to fall back on.
  • Suggestion: have chatwoot.js add document.documentElement.classList.add("chatwoot-ready") on the SDK's ready event, and scope the offset to .chatwoot-ready .theme-back-to-top-button.

3. PR description no longer matches the diff

  • Body says only bottom: 104px !important was added. Diff also sets width, height, right.
  • The compiled-CSS snippet under "How Has This Been Tested?" is from the old diff and does not include the three new declarations.
  • Either update the body or re-run the verification, otherwise the review evidence is stale.

4. scroll boots Chatwoot for effectively every reader

  • scroll is in INTERACTION_EVENTS (src/metaPixelRouteTracker.js:23), so on a docs site every session loads the SDK and opens a persistent websocket.
  • This lands directly on top of fd27e0f5, a Core Web Vitals PR.
  • You already flagged the landing/docs divergence. Taking a position: align docs with landing, gate Chatwoot on pointerdown / keydown / touchstart only. Split INTERACTION_SCRIPTS into two lists rather than moving scroll out globally, since Clarity genuinely wants scroll.

MINOR

5. No window.chatwootSettings

  • colorMode.disableSwitch is false in docusaurus.config.js:200, so docs has a dark theme. The widget will render light in it.
  • Setting window.chatwootSettings = { position: "right", type: "standard", darkMode: "auto" } before run() also makes the CSS assumption about position: right explicit instead of implicit.

6. No re-entry guard in chatwoot.js

  • run() twice creates two bubbles and two sockets. The loader guards it today, but the file does not guard itself.
  • clarity.js next door has its own c[a] = c[a] || ... guard. One line: if (window.$chatwoot) return; inside onload.

7. Stale comment in docusaurus.config.js

  • docusaurus.config.js:549-554 still reads Clarity + Apollo -> lazy, on first user interaction.
  • You updated the same list in metaPixelRouteTracker.js but not this one.

8. The button grew, the arrow did not

  • Theme draws the chevron via ::after with mask: ... 50% / 2rem 2rem, a fixed 32px glyph.
  • Going 48px to 64px drops the glyph from 67% to 50% of the button. It will read as a small arrow in a big circle.
  • Either add mask-size / background-size or drop the width/height override and keep only bottom.

9. Adjacent dead CSS in the block you touched

  • The theme component renders no <svg> child, so .theme-back-to-top-button svg { fill: white } never matches.
  • color: white !important is also inert; the arrow color comes from --ifm-color-emphasis-1000 on ::after.
  • Pre-existing, not yours, but you are editing this exact rule so it is cheap to clean up.

NOTES, NO ACTION

  • The open Chatwoot panel (~400x590) covers the 104px slot, so back-to-top is unreachable while chatting. Acceptable tradeoff.
  • Permissions-Policy: microphone=() blocks Chatwoot voice notes. Irrelevant for text support, noting for the record.

AGREED, CLOSING OUT

  • Plaintext websiteToken: correct, it is a public client identifier like a GA measurement ID. No env var needed.
  • !important on bottom: justified, the theme's CSS module is bundled after custom.css.
  • Optional chaining in static/: consistent with apollo-init.js.
  • No interaction gate inside chatwoot.js: right call, one gate is better than two.

PRAISE

  • Reusing the existing INTERACTION_SCRIPTS loader instead of adding a third injection path.
  • The 22-line comment explaining the 104px math is the kind of thing that saves the next person an hour.
  • Pre-empting reviewer questions in the body is good practice, even where one of the claims turned out wrong.

To clear: fix 1, resolve 2 and 4 either way (change or reasoned decline), refresh 3. 5 to 9 are yours to take or leave.

@amaan-bhati

amaan-bhati commented Sep 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Response to Iteration 1

Pushed as c35333ca. All 9 items addressed: 7 fixed, 2 fixed with a variation. PR body rewritten.

BLOCKING

1. CSP: fixed, and you were right, I was wrong

I checked netlify.toml and static/_headers and concluded there was no CSP. I never checked vercel.json. The claim in the body was wrong and is now removed.

Added to vercel.json:

Directive Added Why
script-src https://chatwoot.keploy.io loads /packs/js/sdk.js
connect-src https://chatwoot.keploy.io wss://chatwoot.keploy.io ActionCable socket
frame-src https://chatwoot.keploy.io widget panel iframe
style-src https://chatwoot.keploy.io see below
font-src https://chatwoot.keploy.io see below

Two beyond your list. 'unsafe-inline' covers an injected <style> but not a <link rel=stylesheet> to the Chatwoot origin, and the landing page's production CSP allows Chatwoot in both style-src and font-src. That allowlist was built from real violations on the same widget, so I matched it rather than waiting to find out. img-src 'self' data: https: already covers it, agreed, left alone.

Diff is one line; I reformatted the whole file on the first attempt and reverted it.

IMPORTANT

2. Unconditional offset: fixed, as suggested

chatwoot.js now adds .chatwoot-ready to <html> on the SDK's chatwoot:ready event, and every geometry declaration is scoped to it. SDK down, blocked or 404s → no class, and the button keeps the theme's own position. No empty gap.

Side benefit: .chatwoot-ready .theme-back-to-top-button is two classes, so it beats the theme's single-class module on specificity and the !important you signed off on earlier is no longer needed. Dropped.

3. Stale body: fixed

Rewritten against the actual diff, with the real compiled CSS from this build, the new file list and the loading-strategy table.

4. scroll booting Chatwoot: fixed, taking your position

Split into two tiers rather than moving scroll globally, as you suggested:

Tier Trigger Scripts
Analytics pointerdown / keydown / touchstart / scroll Clarity, Apollo
Support pointerdown / keydown / touchstart Chatwoot

Clarity keeps scroll untouched. Docs and landing now gate the widget identically.

MINOR

5. chatwootSettings: added

{ position: "right", type: "standard", darkMode: "auto" }, set before run(). position being explicit is the real win, since the CSS assumes it.

One caveat worth recording: darkMode: "auto" follows prefers-color-scheme, not the Docusaurus toggle, and Chatwoot exposes no hook for the latter. A reader on a light OS who flips docs to dark still gets a light widget. Better than always-light, not a full fix.

6. Re-entry guard: added

if (window.$chatwoot) return; at the top, matching the self-guarding style of clarity.js next door.

7. Stale config comment: fixed

docusaurus.config.js now documents the Chatwoot tier and says explicitly that skipping scroll is deliberate.

8. Glyph: fixed, kept the 64px

Went with mask-size rather than dropping the size override, because matching the bubble was a deliberate ask. 2.5rem puts the glyph at 62.5% of the button, near the theme's original 67%. Worth an eyeball on the preview.

9. Dead CSS: removed

Dropped .theme-back-to-top-button svg { fill: white } (theme renders no <svg>, it is an ::after mask) and the inert color: white.

While there: the arrow is painted from --ifm-color-emphasis-1000, which is black in light mode, so it is a black chevron on an orange button. The dead svg rule suggests white was the intent. Pre-existing and a visual change beyond this PR's scope, so I left it. Happy to take it as a follow-up.

Notes acknowledged

  • Open panel covering the 104px slot: agreed, acceptable.
  • microphone=() blocking voice notes: agreed, irrelevant for text support.

Verification: npm run build exit 0, prettier --check clean on all five files, compiled CSS and CSP confirmed in the emitted output. Not merging.

@amaan-bhati
fix: add Chatwoot to CSP, gate back-to-top offset on SDK load, drop s…
c0bbc7c 
…croll trigger

Signed-off-by: amaan-bhati <amaanbhati49@gmail.com>
@amaan-bhati

amaan-bhati commented Sep 21, 2026

Copy link
Copy Markdown
Member Author

Code Review Agent, Iteration 2

Commit reviewed: c0bbc7cb
Scope: 5 files
CI: run-lint pass, prettier pass, DCO pass, asset-budget pass, deploy-preview pass

VERDICT: APPROVE

All 9 items from iteration 1 are resolved. 0 blocking, 0 important, 2 minor notes, neither worth holding the merge for.

ITERATION 1 FOLLOW-UP

# Item Status
1 CSP missing Chatwoot Fixed
2 Unconditional back-to-top offset Fixed
3 Stale PR description Fixed
4 scroll booting Chatwoot Fixed
5 No chatwootSettings Fixed
6 No re-entry guard Fixed, see note A
7 Stale config comment Fixed
8 Glyph shrank with the button Fixed
9 Dead CSS Fixed

VERIFIED AGAINST THE LIVE SDK

Pulled https://chatwoot.keploy.io/packs/js/sdk.js (HTTP 200, 21,995 bytes) and read the injected stylesheet and boot path rather than taking the numbers on trust.

Geometry is exact. The SDK ships .woot-widget-bubble{bottom:20px;height:64px;width:64px;z-index:2147483000!important} and .woot-elements--right{right:20px}. So 20 + 64 + 20 = 104 is right, and at equal widths both centrelines land 52px from the edge. The comment block matches the shipped CSS line for line.

Mobile holds. The only bubble rule under @media (max-width:667px) is .woot-widget-bubble.woot--close{bottom:60px;opacity:0;visibility:hidden}, which applies to the open state where the bubble is hidden anyway. Closed-state geometry is unchanged on small screens, so the 104px stack is correct there too.

The .chatwoot-ready gate has no visible gap. I went looking for a window where the bubble is painted but the class is not yet set, which would reproduce the click-swallowing this PR fixes. There isn't one. In the SDK's loaded handler, onLoad() (which creates the bubble holder) and the chatwoot:ready dispatch are in the same synchronous block, and the holder is created hidden before that. Bubble and class appear together. Good choice of signal.

Specificity claim is correct. .chatwoot-ready .theme-back-to-top-button is (0,2,0) against the theme module's (0,1,0), so it wins on specificity regardless of bundle order and the !important is genuinely unnecessary. Same for the ::after rule, and the mask-size longhand beats the theme's mask shorthand on the same grounds.

Two-tier loader is sound. Handlers are per tier, each tier removes only its own listeners, removeEventListener matches correctly since passive is not part of the match criteria, and armEngagement always runs before any loadTier because it is on the !previousLocation branch. No SSR leak: loadTier bails on typeof window === "undefined", so the module-level loaded flags are never mutated during a Node render.

MINOR, NO ACTION REQUIRED

A. The re-entry guard is redundant, and not in the position its comment describes

The comment says the guard makes the file "safe on its own", but:

  • It sits at the top of the IIFE, which runs at parse time. Two injections in the same tick both clear it before either sdk.js has loaded, so both still call run(). The case the comment names is not the case it covers.
  • It does not matter, because run() opens with if(window.$chatwoot)return; in the shipped SDK. Chatwoot already self-guards. onLoad() separately bails on if(a.getBubbleHolder().length)return;.

So: harmless, already covered upstream. If you want the comment to be literally true, move the check into onload just before run(). Not worth a commit on its own.

B. style-src and font-src additions are inert for this widget

You added https://chatwoot.keploy.io to both, citing violations seen on landing. Against this SDK they do nothing:

  • Styles go in through document.createElement("style") + innerHTML, which 'unsafe-inline' already covers. There is no <link rel=stylesheet> anywhere in the bundle.
  • No @font-face, no .woff reference in the bundle at all.
  • The panel is an iframe, and an iframe document is governed by the CSP on its own response, not the parent's. Parent style-src and font-src never reach it.

Two extra origins on a first-party subdomain is not a real attack surface, so keep them for parity with landing if you prefer. Just worth recording that the justification does not hold here, so nobody treats it as evidence later.

PRAISE

  • Fixing the CSP where it actually lives rather than arguing the original claim.
  • .chatwoot-ready came out better than suggested. Gating on the SDK also let you drop the !important, which was not part of the ask.
  • Splitting into tiers instead of moving scroll globally kept Clarity's trigger intact. That was the right read.
  • Recording the darkMode: "auto" caveat in the body instead of quietly shipping it as a full fix.
  • Verification section quotes the real compiled output, so it is checkable.

Approved. Notes A and B are optional. Nothing here blocks merge.

@amaan-bhati
fix: move Chatwoot re-entry guard into onload and drop inert style/fo…
1be5b0b 
…nt CSP entries

Signed-off-by: amaan-bhati <amaanbhati49@gmail.com>
@amaan-bhati

amaan-bhati commented Sep 21, 2026

Copy link
Copy Markdown
Member Author

Code Review Agent, Iteration 2

Commit reviewed: c0bbc7cb Scope: 5 files CI: run-lint pass, prettier pass, DCO pass, asset-budget pass, deploy-preview pass

VERDICT: APPROVE

All 9 items from iteration 1 are resolved. 0 blocking, 0 important, 2 minor notes, neither worth holding the merge for.

ITERATION 1 FOLLOW-UP

Item Status

1 CSP missing Chatwoot Fixed
2 Unconditional back-to-top offset Fixed
3 Stale PR description Fixed
4 scroll booting Chatwoot Fixed
5 No chatwootSettings Fixed
6 No re-entry guard Fixed, see note A
7 Stale config comment Fixed
8 Glyph shrank with the button Fixed
9 Dead CSS Fixed

VERIFIED AGAINST THE LIVE SDK

Pulled https://chatwoot.keploy.io/packs/js/sdk.js (HTTP 200, 21,995 bytes) and read the injected stylesheet and boot path rather than taking the numbers on trust.

Geometry is exact. The SDK ships .woot-widget-bubble{bottom:20px;height:64px;width:64px;z-index:2147483000!important} and .woot-elements--right{right:20px}. So 20 + 64 + 20 = 104 is right, and at equal widths both centrelines land 52px from the edge. The comment block matches the shipped CSS line for line.

Mobile holds. The only bubble rule under @media (max-width:667px) is .woot-widget-bubble.woot--close{bottom:60px;opacity:0;visibility:hidden}, which applies to the open state where the bubble is hidden anyway. Closed-state geometry is unchanged on small screens, so the 104px stack is correct there too.

The .chatwoot-ready gate has no visible gap. I went looking for a window where the bubble is painted but the class is not yet set, which would reproduce the click-swallowing this PR fixes. There isn't one. In the SDK's loaded handler, onLoad() (which creates the bubble holder) and the chatwoot:ready dispatch are in the same synchronous block, and the holder is created hidden before that. Bubble and class appear together. Good choice of signal.

Specificity claim is correct. .chatwoot-ready .theme-back-to-top-button is (0,2,0) against the theme module's (0,1,0), so it wins on specificity regardless of bundle order and the !important is genuinely unnecessary. Same for the ::after rule, and the mask-size longhand beats the theme's mask shorthand on the same grounds.

Two-tier loader is sound. Handlers are per tier, each tier removes only its own listeners, removeEventListener matches correctly since passive is not part of the match criteria, and armEngagement always runs before any loadTier because it is on the !previousLocation branch. No SSR leak: loadTier bails on typeof window === "undefined", so the module-level loaded flags are never mutated during a Node render.

MINOR, NO ACTION REQUIRED

A. The re-entry guard is redundant, and not in the position its comment describes

The comment says the guard makes the file "safe on its own", but:

  • It sits at the top of the IIFE, which runs at parse time. Two injections in the same tick both clear it before either sdk.js has loaded, so both still call run(). The case the comment names is not the case it covers.
  • It does not matter, because run() opens with if(window.$chatwoot)return; in the shipped SDK. Chatwoot already self-guards. onLoad() separately bails on if(a.getBubbleHolder().length)return;.

So: harmless, already covered upstream. If you want the comment to be literally true, move the check into onload just before run(). Not worth a commit on its own.

B. style-src and font-src additions are inert for this widget

You added https://chatwoot.keploy.io to both, citing violations seen on landing. Against this SDK they do nothing:

  • Styles go in through document.createElement("style") + innerHTML, which 'unsafe-inline' already covers. There is no <link rel=stylesheet> anywhere in the bundle.
  • No @font-face, no .woff reference in the bundle at all.
  • The panel is an iframe, and an iframe document is governed by the CSP on its own response, not the parent's. Parent style-src and font-src never reach it.

Two extra origins on a first-party subdomain is not a real attack surface, so keep them for parity with landing if you prefer. Just worth recording that the justification does not hold here, so nobody treats it as evidence later.

PRAISE

  • Fixing the CSP where it actually lives rather than arguing the original claim.
  • .chatwoot-ready came out better than suggested. Gating on the SDK also let you drop the !important, which was not part of the ask.
  • Splitting into tiers instead of moving scroll globally kept Clarity's trigger intact. That was the right read.
  • Recording the darkMode: "auto" caveat in the body instead of quietly shipping it as a full fix.
  • Verification section quotes the real compiled output, so it is checkable.

Approved. Notes A and B are optional. Nothing here blocks merge.

Both notes are now fixed rather than accepted. Pushed as 1be5b0bd. I pulled the same bundle (HTTP 200, 21,995 bytes) and confirmed every claim before changing anything.

Note A: guard moved into onload

Your diagnosis was exact. At parse time $chatwoot is undefined for every injection, so two in one tick both passed and the comment described a protection the code did not provide.

Moved the check into onload, immediately before run(). By then the first run() has set $chatwoot, so a second injection actually stops. The comment now says why it lives there instead of at the top.

Moved rather than deleted, even though you are right that it is redundant. run()'s internal if(window.$chatwoot)return and onLoad()'s getBubbleHolder().length check are both undocumented SDK internals. One line that does not depend on them staying true is worth keeping.

Note B: inert directives removed

You are right, and my justification was wrong. I claimed landing's allowlist "was built from real violations on the same widget". That was an assumption about why those origins existed, not something I checked. Against this bundle:

  • 0 occurrences of rel="stylesheet" or createElement("link")
  • 0 occurrences of @font-face, .woff or .ttf
  • 1 createElement("style"), which 'unsafe-inline' already covers

And the iframe point is decisive: the panel document is governed by the CSP on its own response, so parent style-src and font-src never reach it regardless of what it loads.

Rather than keep them for parity, I removed both. A CSP should list what the page actually needs, and two origins added on a false premise are two origins the next person has to re-derive. The header is back to style-src 'self' 'unsafe-inline' and font-src 'self' data:.

Chatwoot now appears in exactly three directives, each with a reason:

Directive Origin Why
script-src https://chatwoot.keploy.io loads /packs/js/sdk.js
connect-src https://chatwoot.keploy.io, wss://chatwoot.keploy.io ActionCable socket
frame-src https://chatwoot.keploy.io widget panel iframe

PR description corrected

The body claimed five directives and cited the violations justification. Both are now fixed, so the reasoning in the description matches the diff and nobody cites the wrong premise later.

Verification

npm run build exit 0, prettier --check clean, vercel.json still valid JSON, CSP diff still one line.

Thanks for checking the geometry against the shipped CSS instead of trusting the comment. Confirming .woot-widget-bubble{bottom:20px;height:64px;width:64px} and .woot-elements--right{right:20px} from source is exactly the check I should have done when I wrote the numbers down.

@amaan-bhati

amaan-bhati commented Sep 21, 2026

Copy link
Copy Markdown
Member Author

Code Review Agent, Iteration 3

Commit reviewed: 1be5b0bd
Delta from iteration 2: 2 files, static/scripts/chatwoot.js and vercel.json
CI: run-lint pass, prettier pass, DCO pass, asset-budget pass, deploy-preview pass

VERDICT: APPROVE

0 blocking, 0 important, 0 minor. Nothing outstanding. Both iteration 2 notes were optional and both are now closed.

FOLLOW-UP

Note Status
A. Guard in the wrong place Fixed, moved into onload
B. Inert style-src / font-src Fixed, both removed

VERIFIED

A. Guard placement is now correct. The check sits immediately before run(), so on a same-tick double injection the first onload sets $chatwoot and the second returns. That is the case the old placement could not catch. Keeping it despite run() self-guarding is the right call, since that guard is an undocumented SDK internal and this does not depend on it.

Traced the failure paths too: if chatwootSDK is undefined the optional chaining no-ops, $chatwoot is never set, .chatwoot-ready is never added, and the button keeps the theme's geometry. Double injection registers two chatwoot:ready listeners, but classList.add is idempotent. No path leaves the page in a broken state.

B. Removal is safe. Re-ran the checks independently against the same bundle rather than taking the numbers from the reply:

rel="stylesheet"          0
rel='stylesheet'          0
createElement("link")     0
@font-face                0
.woff                     0
.ttf                      0
createElement("style")    1

Full element census in the SDK: button x2, div x3, iframe x1, span x1, style x1. No link node exists, so style-src and font-src had nothing to authorise. Removing them is correct, not just tidier.

CSP is now minimal and each entry is load-bearing. script-src for the bundle, connect-src plus wss: for the socket, frame-src for the panel. vercel.json parses, diff is still one line.

Body matches the diff. Line 68 records why style-src and font-src are deliberately untouched, so the next person does not re-add them on the old premise.

PRAISE

Removing the two directives rather than keeping them for parity was the better call, and worth naming: parity with a policy nobody has verified just propagates the unverified part. Same for correcting the "built from real violations" claim rather than leaving it in the thread.

Approved, and done reviewing. Three iterations, 11 items raised, 11 closed. Ready to merge.

@amaan-bhati

amaan-bhati commented Sep 21, 2026

Copy link
Copy Markdown
Member Author

Code Review Agent, Iteration 3

Commit reviewed: 1be5b0bd Delta from iteration 2: 2 files, static/scripts/chatwoot.js and vercel.json CI: run-lint pass, prettier pass, DCO pass, asset-budget pass, deploy-preview pass

VERDICT: APPROVE

0 blocking, 0 important, 0 minor. Nothing outstanding. Both iteration 2 notes were optional and both are now closed.

FOLLOW-UP

Note Status
A. Guard in the wrong place Fixed, moved into onload
B. Inert style-src / font-src Fixed, both removed

VERIFIED

A. Guard placement is now correct. The check sits immediately before run(), so on a same-tick double injection the first onload sets $chatwoot and the second returns. That is the case the old placement could not catch. Keeping it despite run() self-guarding is the right call, since that guard is an undocumented SDK internal and this does not depend on it.

Traced the failure paths too: if chatwootSDK is undefined the optional chaining no-ops, $chatwoot is never set, .chatwoot-ready is never added, and the button keeps the theme's geometry. Double injection registers two chatwoot:ready listeners, but classList.add is idempotent. No path leaves the page in a broken state.

B. Removal is safe. Re-ran the checks independently against the same bundle rather than taking the numbers from the reply:

rel="stylesheet"          0
rel='stylesheet'          0
createElement("link")     0
@font-face                0
.woff                     0
.ttf                      0
createElement("style")    1

Full element census in the SDK: button x2, div x3, iframe x1, span x1, style x1. No link node exists, so style-src and font-src had nothing to authorise. Removing them is correct, not just tidier.

CSP is now minimal and each entry is load-bearing. script-src for the bundle, connect-src plus wss: for the socket, frame-src for the panel. vercel.json parses, diff is still one line.

Body matches the diff. Line 68 records why style-src and font-src are deliberately untouched, so the next person does not re-add them on the old premise.

PRAISE

Removing the two directives rather than keeping them for parity was the better call, and worth naming: parity with a policy nobody has verified just propagates the unverified part. Same for correcting the "built from real violations" claim rather than leaving it in the thread.

Approved, and done reviewing. Three iterations, 11 items raised, 11 closed. Ready to merge.

Nothing outstanding on my side either. Noting two things from your verification that are worth keeping in the thread.

The failure-path trace is the part I had not written down anywhere. If chatwootSDK is undefined the optional chaining no-ops, $chatwoot is never set, .chatwoot-ready is never added, and the button keeps the theme's geometry. That is the behaviour the .chatwoot-ready gate was added for in iteration 1, and it is worth having it confirmed by tracing rather than by assertion.

The element census is a better check than the greps I ran. Counting the nodes the SDK can actually create (button x2, div x3, iframe x1, span x1, style x1, no link) proves the absence, where my greps only failed to find the presence. Same conclusion, stronger evidence.

On parity: agreed, and it is the part I would have got wrong without the push back. Copying an allowlist because another property has it carries over the unverified reasoning along with the entries, and the entries are the easy half to review.

Final state on 1be5b0bd: six checks green, tree clean, CSP down to three load-bearing directives, body matching the diff.

nehagup
nehagup reviewed Sep 23, 2026
View reviewed changes

@nehagup nehagup left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work. The geometry reasoning and the .chatwoot-ready gate are well done. A few things from a fresh pass:

  1. The CSP change probably doesn't reach production. The live keploy.io/docs is served from S3/CloudFront, and the content-security-policy-report-only header it actually returns is not the one in vercel.json. The live header has form-action, 'unsafe-eval' and fonts.googleapis.com, which the vercel.json value doesn't. So this edit likely only affects Vercel previews, and the CloudFront response-headers policy needs chatwoot.keploy.io / wss://chatwoot.keploy.io added too. Nothing breaks either way because it's Report-Only, but let's keep the two in sync, or delete the one that isn't used.
  2. Tag where the conversation came from. Docs and landing share one websiteToken, so support can't tell a docs chat from a landing-page chat. On chatwoot:ready, call window.$chatwoot.setCustomAttributes({ source: "docs" }). Chatwoot also records the page URL, but an attribute is easier to filter and route by.
  3. Product question on the gate. Chatwoot only loads after a click, keypress or tap. Readers who land from Google, scroll, read and leave never see the bubble, and on docs that's a large share of visitors. The websocket-cost argument is fair. Just confirm we're OK with that trade-off, and consider checking in two weeks how many docs sessions actually loaded the widget.
  4. Cookie policy. Chatwoot sets its own cookies (cw_conversation, etc.). Landing already runs it, so the policy probably covers it already. Worth a quick check that the cookie policy lists it.

None of these block merging except 1, which is a quick fix.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nehagup nehagup nehagup left review comments
Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@amaan-bhati @nehagup