Open
Conversation
Replace Intercom chat widget with Pylon across the editor, debugger, help menus, and error modals. Add bootPylon utility with display name fallback (name -> username -> email) so Pylon SDK accepts identity. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…o test-pylon-ai
Revert app/client/src/ee/configs/index.ts to keep EE files untouched in this PR. Move the Pylon/pylon Window type declarations to app/client/src/ce/configs/index.ts so the migration stays self-contained in CE. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughReplaces Intercom with Pylon for in‑app chat: environment variables, CI/build templates, client bootstrap, TypeScript configs/types, boot/update utilities, React UI hooks/components, server-side HMAC identity support, and related tests/documentation updated or added. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant NGINX as Proxy/NGINX
participant Client as App Client JS
participant Server as Appsmith Server
participant PylonCDN as Pylon CDN
Browser->>NGINX: GET index.html
NGINX-->>Browser: Serve HTML with injected PYLON_APP_ID / DISABLE_PYLON
Browser->>Client: Client JS parses config, sets window.pylon.chat_settings.app_id
Client->>Server: Optionally fetch current user (includes emailVerificationHash)
Server-->>Client: Responds with user.emailVerificationHash (or null)
Client->>PylonCDN: Inject/Load widget script (conditional)
PylonCDN-->>Browser: Exposes window.Pylon
Client->>Browser: bootPylon(user) -> window.pylon.chat_settings.email_hash set
Client->>PylonCDN: window.Pylon("setNewIssueCustomFields"/"show"/"hide"...)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/24112770987. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/client/src/pages/Editor/EditorName/useNavigationMenuData.ts (1)
171-180:⚠️ Potential issue | 🟠 MajorDon't render a chat action that can only no-op.
isVisiblenow depends only onisPylonChatAvailable(), butonClickstill requirescloudHosting || isIntercomConsentGiven. On self-hosted instances without consent, users will see Chat with us and nothing will happen when they click it. Either gate visibility with the same condition or route this entry point through the consent flow used in the homepage help menu.💡 Minimal local fix
{ startIcon: "chat-help", text: "Chat with us", onClick: () => { if (cloudHosting || isIntercomConsentGiven) { window.Pylon("show"); } }, type: MenuTypes.MENU, - isVisible: isPylonChatAvailable(), + isVisible: + isPylonChatAvailable() && + (cloudHosting || isIntercomConsentGiven), },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/client/src/pages/Editor/EditorName/useNavigationMenuData.ts` around lines 171 - 180, The "Chat with us" menu item is visible when isPylonChatAvailable() but its onClick no-ops unless cloudHosting || isIntercomConsentGiven, causing a clickable no-op on self-hosted installs; update the visibility or the click handler to be consistent: either change the menu item's isVisible to isPylonChatAvailable() && (cloudHosting || isIntercomConsentGiven) so it only renders when the click will work, or route the onClick through the existing consent flow (same flow used by the homepage help menu) when !cloudHosting && !isIntercomConsentGiven so the user is prompted for consent before calling window.Pylon("show"); locate the "Chat with us" Menu entry and adjust its isVisible or onClick accordingly.app/client/src/pages/Editor/FirstTimeUserOnboarding/HelpMenu.tsx (1)
89-98:⚠️ Potential issue | 🟠 MajorPopulate Pylon metadata on the onboarding chat path.
This branch skips
updatePylonChatIdentity()and goes straight towindow.Pylon("show"). Users who already cleared consent will open chat without theinstance_id,license_id, and version fields that the new helper sets.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/client/src/pages/Editor/FirstTimeUserOnboarding/HelpMenu.tsx` around lines 89 - 98, The onboarding branch for the "pylon-trigger" item calls window.Pylon("show") without first populating Pylon metadata, so users who already gave consent open chat missing instance_id/license_id/version; modify the branch where item.id === "pylon-trigger" to call updatePylonChatIdentity() and await its completion (or ensure it runs) before calling window.Pylon("show") when isPylonChatAvailable() and (user?.isIntercomConsentGiven || cloudHosting) are true, while keeping the existing props.setShowIntercomConsent(true) flow for the consent-needed path.
🧹 Nitpick comments (2)
docs/analysis/share-from-app-datasource-issues.md (1)
152-158: “CONFIRMED” wording is too strong without attached proof artifacts.Consider changing “CONFIRMED” to “likely” (or linking concrete evidence like logs/HAR/Redux snapshots inline) to keep the analysis precise.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/analysis/share-from-app-datasource-issues.md` around lines 152 - 158, Update the verdict language in the "Hypothesis 1" heading and body: replace the all-caps "CONFIRMED" label with a softer term like "likely" (or add links/references to concrete evidence such as logs, HAR files, or Redux snapshots) to avoid overclaiming; ensure any inline text that asserts confirmation references the evidence or is reworded (e.g., "likely — evidence: [link/log]") and keep the technical fix guidance about calling fetchRolesForWorkspace with the application's workspaceId and resetting workspaceRoles when workspace context changes unchanged.app/client/cypress/support/Pages/DebuggerHelper.ts (1)
37-37: Prefer adata-testidfor the chat option.
#pylon-triggercouples the test to a DOM id. If this trigger is Appsmith-owned UI, expose a stabledata-*selector and point the page object at that instead.As per coding guidelines, "Use data-* attributes for selectors."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/client/cypress/support/Pages/DebuggerHelper.ts` at line 37, The selector _pylonChatOption currently points at the DOM id "#pylon-trigger", which couples tests to an unstable id; change it to use a stable data attribute (e.g., data-testid or data-cy) exposed by the app instead. Update the _pylonChatOption entry in DebuggerHelper (the _pylonChatOption constant) to target the new data-* selector string (for example '[data-testid="pylon-trigger"]') and coordinate with the app team to ensure the element includes the chosen data-* attribute.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Around line 55-57: The dotenv-linter flags an UnorderedKey because
APPSMITH_PYLON_APP_ID and APPSMITH_DISABLE_PYLON are out of alphabetical order;
reorder the two keys so APPSMITH_DISABLE_PYLON appears before
APPSMITH_PYLON_APP_ID (preserving their values/comments) to satisfy the linter
and avoid the UnorderedKey warning.
In `@app/client/public/index.html`:
- Around line 180-194: PYLON_APP_ID_RAW can be boolean false (parseConfig
returns false) so PYLON_APP_ID may become boolean and calling .length throws;
update the bootstrap to normalize the value before checking length by coercing
PYLON_APP_ID_RAW to a string or providing a string fallback (e.g., compute
PYLON_APP_ID_RAW = String(parseConfig(...) || "") or set PYLON_APP_ID =
AIRGAPPED ? "" : (String(PYLON_APP_ID_RAW || ""))), then use PYLON_APP_ID.length
or a simple truthy check; apply the same normalization wherever PYLON_APP_ID is
read (also the other occurrence referenced around the later check) and keep
DISABLE_PYLON handling unchanged.
In `@app/client/src/components/editorComponents/Debugger/ContextualMenu.tsx`:
- Around line 99-114: The CHAT_WIDGET context menu handler
(CONTEXT_MENU_ACTIONS.CHAT_WIDGET) currently calls window.Pylon when
isPylonChatAvailable() is true and bypasses the consent gate; update the
onSelect flow to first check the same consent condition used elsewhere
(cloudHosting || isIntercomConsentGiven) or dispatch the shared consent flow
before invoking window.Pylon("showNewMessage", ...) and window.Pylon("show");
ensure you reference the existing consent check used in other Pylon entry points
so self-hosted users cannot open the widget unless consent is granted or the
consent flow completes.
- Around line 38-61: getOptions currently can return a single
CONTEXT_MENU_ACTIONS.CHAT_WIDGET which later gets filtered out (when Pylon is
unavailable) leaving an empty menu trigger; update the code path that calls
getOptions in ContextualMenu.tsx to immediately filter the returned array
(remove unavailable actions) and if the resulting list is empty, do not render
the menu/trigger at all (or alternatively insert a fallback such as
CONTEXT_MENU_ACTIONS.DOCS) so users cannot open a blank popover; reference
getOptions, CONTEXT_MENU_ACTIONS, and the render-time filter logic and ensure
the empty-check happens before any JSX that renders the popover/trigger.
In `@app/client/src/pages/Editor/HelpButton.tsx`:
- Around line 300-305: The Pylon chat open path skips pushing metadata for
returning/cloud-hosted users before calling window.Pylon("show"), so update the
branch in the pylon-trigger handler (the if block guarded by
isPylonChatAvailable() and user?.isIntercomConsentGiven || cloudHosting) to
invoke the same metadata-sync routine used in the consent flow (the function
that pushes instance_id, license_id, and appsmith_version to Pylon — e.g.,
syncPylonMetadata / pushPylonMetadata) immediately before calling
window.Pylon("show"); ensure you call that metadata sync with the same
payload/keys as the consent path so instance_id, license_id, and
appsmith_version are sent prior to showing the chat.
- Around line 225-227: The effect currently calls bootPylon(user) but only
depends on user?.email, so updates to user.name or user.username won't re-run
and Pylon can have stale identity; change the dependency to either the whole
user object (useEffect(..., [user])) or explicitly include all identity fields
(useEffect(..., [user?.name, user?.username, user?.email])) so bootPylon(user)
runs whenever any identity field changes.
In `@app/client/src/utils/bootPylon.ts`:
- Around line 47-57: bootPylon() currently overwrites window.pylon.chat_settings
with raw username/displayName, losing the boot-time account linkage
(account_external_id and the filtered ANONYMOUS_USERNAME handling); change
bootPylon to preserve existing boot-time identifiers by merging instead of
replacing: when setting window.pylon.chat_settings reuse an existing
window.pylon.chat_settings.account_external_id (or compute with the same helper
if missing) and reuse the filtered username logic (respect ANONYMOUS_USERNAME)
rather than writing the raw username; update the assignment of window.pylon to
merge existing chat_settings (account_external_id, filtered username) with the
new values so consent refreshes keep the original account_external_id and
anonymity filtering.
In `@deploy/heroku/README.MD`:
- Around line 39-40: The section header "Intercom:" is inconsistent with the
Pylon-related variable; update the header to reflect Pylon (e.g., change
"Intercom:" to "Pylon:" or similar) so it matches the described setting
APPSMITH_DISABLE_PYLON and its description about disabling the Pylon chat
widget.
In `@docs/analysis/multi-file-upload-limits.md`:
- Around line 114-115: The doc's two limits contradict each other; update the
wording in multi-file-upload-limits.md to clarify that the widget supports a
per-file client-side limit of 200 MB but the default server-side total request
cap is 150 MB (configurable), so uploads larger than the server cap will be
rejected unless the server config is changed; alternatively, align the numbers
by changing the per-file max to 150 MB if you intend the server cap to be the
effective limit. Make this change near the existing bullet points that currently
read "Per-file max: 200 MB (widget)" and "Total request max: 150 MB (server,
configurable)" so readers understand which limit applies and which one is
enforced.
- Around line 1-130: This change adds a documentation analysis unrelated to the
Intercom→Pylon migration; remove or relocate
docs/analysis/multi-file-upload-limits.md from this PR and put it into a
separate docs-only PR (or a branch) so the migration release remains focused.
Specifically, revert or remove the added file from the current branch/commit,
create a new branch/PR containing the file, and update the current PR
description to state that docs were intentionally excluded (or reference the new
docs PR). Use the file name (multi-file-upload-limits.md) and references in the
doc (e.g., FilePickerWidgetV2, FilepickerWidget, MultiFilePickerControl) when
creating the separate PR to preserve context.
In `@docs/analysis/share-from-app-datasource-issues.md`:
- Around line 176-191: The document duplicates the same recommendation to
clear/reset workspaceRoles on workspace context change; remove one of the two
entries so there is a single canonical instruction. Specifically, keep the
recommendation that best fits the flow (e.g., the one tied to
SET_CURRENT_WORKSPACE_ID and clearing state.ui.workspaces.workspaceRoles to []
and triggering fetchAllRoles(fetchRolesForWorkspace) for the InviteUsersForm),
and delete the other duplicate paragraph that references
workspaceRoles/fetchRolesForWorkspace so only one clear action is documented
alongside related symbols like SET_CURRENT_WORKSPACE_ID, fetchWorkspace,
fetchRolesForWorkspace, fetchAllRoles, workspaceRoles, currentWorkspace and
InviteUsersForm.
- Around line 1-341: This PR contains an out-of-scope analysis doc ("Analysis:
Datasource Issues When Sharing App from Within the App (Default Role)") that
should be split out; remove the document from this branch/commit and create a
separate PR that only contains the analysis file, or move the file to a
dedicated issue-analysis PR, ensuring this release/migration PR only includes
migration-related changes; reference the doc title to locate the file and submit
the separate PR with an explanatory description linking back to the original
issue.
In `@docs/plans/2026-03-05-html-lang-customization.md`:
- Around line 392-394: The fenced code block that currently contains the literal
string "app/client/src/ce/configs/index.ts" is missing a language specifier
(MD040); update that markdown block to use a language tag (e.g., change ``` to
```text) so it reads ```text followed by app/client/src/ce/configs/index.ts and
closing ``` to satisfy markdownlint and make the file path display correctly.
- Line 3: Remove the agent-specific execution directive phrase "**For Claude:**
REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan
task-by-task" from the plan and replace it with neutral contributor guidance
such as a general instruction to "Implement this plan task-by-task" or "Follow
the plan step-by-step" so the document contains product/engineering content
only; ensure the edited sentence preserves intent but omits any
assistant/tooling persona or platform-specific "superpowers:executing-plans"
reference.
---
Outside diff comments:
In `@app/client/src/pages/Editor/EditorName/useNavigationMenuData.ts`:
- Around line 171-180: The "Chat with us" menu item is visible when
isPylonChatAvailable() but its onClick no-ops unless cloudHosting ||
isIntercomConsentGiven, causing a clickable no-op on self-hosted installs;
update the visibility or the click handler to be consistent: either change the
menu item's isVisible to isPylonChatAvailable() && (cloudHosting ||
isIntercomConsentGiven) so it only renders when the click will work, or route
the onClick through the existing consent flow (same flow used by the homepage
help menu) when !cloudHosting && !isIntercomConsentGiven so the user is prompted
for consent before calling window.Pylon("show"); locate the "Chat with us" Menu
entry and adjust its isVisible or onClick accordingly.
In `@app/client/src/pages/Editor/FirstTimeUserOnboarding/HelpMenu.tsx`:
- Around line 89-98: The onboarding branch for the "pylon-trigger" item calls
window.Pylon("show") without first populating Pylon metadata, so users who
already gave consent open chat missing instance_id/license_id/version; modify
the branch where item.id === "pylon-trigger" to call updatePylonChatIdentity()
and await its completion (or ensure it runs) before calling window.Pylon("show")
when isPylonChatAvailable() and (user?.isIntercomConsentGiven || cloudHosting)
are true, while keeping the existing props.setShowIntercomConsent(true) flow for
the consent-needed path.
---
Nitpick comments:
In `@app/client/cypress/support/Pages/DebuggerHelper.ts`:
- Line 37: The selector _pylonChatOption currently points at the DOM id
"#pylon-trigger", which couples tests to an unstable id; change it to use a
stable data attribute (e.g., data-testid or data-cy) exposed by the app instead.
Update the _pylonChatOption entry in DebuggerHelper (the _pylonChatOption
constant) to target the new data-* selector string (for example
'[data-testid="pylon-trigger"]') and coordinate with the app team to ensure the
element includes the chosen data-* attribute.
In `@docs/analysis/share-from-app-datasource-issues.md`:
- Around line 152-158: Update the verdict language in the "Hypothesis 1" heading
and body: replace the all-caps "CONFIRMED" label with a softer term like
"likely" (or add links/references to concrete evidence such as logs, HAR files,
or Redux snapshots) to avoid overclaiming; ensure any inline text that asserts
confirmation references the evidence or is reworded (e.g., "likely — evidence:
[link/log]") and keep the technical fix guidance about calling
fetchRolesForWorkspace with the application's workspaceId and resetting
workspaceRoles when workspace context changes unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f60426b1-0aa1-4adc-a443-e53881c06ab5
📒 Files selected for processing (34)
.env.example.github/workflows/ci-test-custom-script.yml.github/workflows/ci-test-limited-with-count.yml.github/workflows/ci-test-limited.yml.github/workflows/github-release.ymlapp/client/.env.exampleapp/client/cypress/e2e/Regression/ClientSide/Editor/HelpButton_spec.tsapp/client/cypress/support/Pages/DebuggerHelper.tsapp/client/docker/templates/nginx-app.conf.templateapp/client/jest.config.jsapp/client/public/index.htmlapp/client/src/ce/configs/index.tsapp/client/src/ce/configs/types.tsapp/client/src/ce/utils/AnalyticsUtil.tsxapp/client/src/components/editorComponents/Debugger/ContextualMenu.tsxapp/client/src/components/editorComponents/Debugger/DebuggerLogs.tsxapp/client/src/components/editorComponents/Debugger/Errors.tsxapp/client/src/git/components/RepoLimitErrorModal/RepoLimitErrorModalView.tsxapp/client/src/pages/AdminSettings/index.tsxapp/client/src/pages/Editor/EditorName/useNavigationMenuData.tsapp/client/src/pages/Editor/FirstTimeUserOnboarding/Checklist.tsxapp/client/src/pages/Editor/FirstTimeUserOnboarding/HelpMenu.tsxapp/client/src/pages/Editor/HelpButton.tsxapp/client/src/pages/Editor/gitSync/RepoLimitExceededErrorModal.tsxapp/client/src/pages/common/PageHeader.tsxapp/client/src/pages/common/SearchBar/HomepageHeaderAction.tsxapp/client/src/pages/common/datasourceAuth/AuthMessage.tsxapp/client/src/utils/bootIntercom.tsapp/client/src/utils/bootPylon.tsapp/server/appsmith-server/pom.xmldeploy/heroku/README.MDdocs/analysis/multi-file-upload-limits.mddocs/analysis/share-from-app-datasource-issues.mddocs/plans/2026-03-05-html-lang-customization.md
💤 Files with no reviewable changes (1)
- app/client/src/utils/bootIntercom.ts
Comment on lines
+55
to
+57
| # Pylon in-app chat (optional runtime override; prefer REACT_APP_PYLON_APP_ID at build) | ||
| APPSMITH_PYLON_APP_ID= | ||
| APPSMITH_DISABLE_PYLON= |
Contributor
There was a problem hiding this comment.
Reorder new env keys to satisfy dotenv-linter.
APPSMITH_DISABLE_PYLON should appear before APPSMITH_PYLON_APP_ID to avoid the UnorderedKey warning.
🔧 Proposed fix
-APPSMITH_PYLON_APP_ID=
APPSMITH_DISABLE_PYLON=
+APPSMITH_PYLON_APP_ID=🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 57-57: [UnorderedKey] The APPSMITH_DISABLE_PYLON key should go before the APPSMITH_PYLON_APP_ID key
(UnorderedKey)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.env.example around lines 55 - 57, The dotenv-linter flags an UnorderedKey
because APPSMITH_PYLON_APP_ID and APPSMITH_DISABLE_PYLON are out of alphabetical
order; reorder the two keys so APPSMITH_DISABLE_PYLON appears before
APPSMITH_PYLON_APP_ID (preserving their values/comments) to satisfy the linter
and avoid the UnorderedKey warning.
Comment on lines
38
to
+61
| const getOptions = (type?: string, subType?: string) => { | ||
| const defaultOptions = [ | ||
| CONTEXT_MENU_ACTIONS.DOCS, | ||
| CONTEXT_MENU_ACTIONS.INTERCOM, | ||
| CONTEXT_MENU_ACTIONS.CHAT_WIDGET, | ||
| ]; | ||
|
|
||
| if (subType) { | ||
| switch (subType) { | ||
| // These types are sent by the server | ||
| case PLUGIN_EXECUTION_ERRORS.DATASOURCE_CONFIGURATION_ERROR: | ||
| return [CONTEXT_MENU_ACTIONS.INTERCOM]; | ||
| return [CONTEXT_MENU_ACTIONS.CHAT_WIDGET]; | ||
| case PLUGIN_EXECUTION_ERRORS.PLUGIN_ERROR: | ||
| return [CONTEXT_MENU_ACTIONS.INTERCOM]; | ||
| return [CONTEXT_MENU_ACTIONS.CHAT_WIDGET]; | ||
| case PLUGIN_EXECUTION_ERRORS.CONNECTIVITY_ERROR: | ||
| return [CONTEXT_MENU_ACTIONS.DOCS]; | ||
| case PLUGIN_EXECUTION_ERRORS.ACTION_CONFIGURATION_ERROR: | ||
| return [CONTEXT_MENU_ACTIONS.DOCS, CONTEXT_MENU_ACTIONS.INTERCOM]; | ||
| return [CONTEXT_MENU_ACTIONS.DOCS, CONTEXT_MENU_ACTIONS.CHAT_WIDGET]; | ||
| default: | ||
| return defaultOptions; | ||
| } | ||
| } else { | ||
| switch (type) { | ||
| case PropertyEvaluationErrorType.VALIDATION: | ||
| return [CONTEXT_MENU_ACTIONS.DOCS, CONTEXT_MENU_ACTIONS.INTERCOM]; | ||
| return [CONTEXT_MENU_ACTIONS.DOCS, CONTEXT_MENU_ACTIONS.CHAT_WIDGET]; |
Contributor
There was a problem hiding this comment.
Avoid rendering an empty contextual menu.
getOptions() now returns CHAT_WIDGET alone for some error types. When Pylon is unavailable, the render-time filter drops that sole item but still leaves the trigger in place, so users can open a blank popover. Filter the option list before rendering and skip the menu entirely when it becomes empty, or add a docs fallback.
Also applies to: 143-146
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/client/src/components/editorComponents/Debugger/ContextualMenu.tsx`
around lines 38 - 61, getOptions currently can return a single
CONTEXT_MENU_ACTIONS.CHAT_WIDGET which later gets filtered out (when Pylon is
unavailable) leaving an empty menu trigger; update the code path that calls
getOptions in ContextualMenu.tsx to immediately filter the returned array
(remove unavailable actions) and if the resulting list is empty, do not render
the menu/trigger at all (or alternatively insert a fallback such as
CONTEXT_MENU_ACTIONS.DOCS) so users cannot open a blank popover; reference
getOptions, CONTEXT_MENU_ACTIONS, and the render-time filter logic and ensure
the empty-check happens before any JSX that renders the popover/trigger.
Comment on lines
225
to
227
| useEffect(() => { | ||
| bootIntercom(user); | ||
| bootPylon(user); | ||
| }, [user?.email]); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Locate and examine the bootPylon function
fd -e ts -e tsx | xargs grep -l "bootPylon" | head -20Repository: appsmithorg/appsmith
Length of output: 3354
🏁 Script executed:
# Read HelpButton.tsx around the mentioned lines
wc -l app/client/src/pages/Editor/HelpButton.tsxRepository: appsmithorg/appsmith
Length of output: 110
🏁 Script executed:
# Search for bootPylon definition
rg "function bootPylon|const bootPylon|export.*bootPylon" -A 10 -B 2Repository: appsmithorg/appsmith
Length of output: 877
🏁 Script executed:
# Read full bootPylon implementation
cat -n app/client/src/utils/bootPylon.tsRepository: appsmithorg/appsmith
Length of output: 2375
🏁 Script executed:
# Read HelpButton.tsx context around lines 225-227
sed -n '210,240p' app/client/src/pages/Editor/HelpButton.tsx | cat -nRepository: appsmithorg/appsmith
Length of output: 970
🏁 Script executed:
# Verify that user.name and user.username are actual fields in the User type
rg "interface User|type User" app/client/src/constants/userConstants.ts -A 15Repository: appsmithorg/appsmith
Length of output: 854
Expand the dependency array to cover all user identity fields.
bootPylon() extracts and passes name, username, and email to Pylon. The effect currently depends only on user?.email, so changes to name or username won't trigger a re-run, leaving the widget with stale identity data. Change the dependency to [user] or list all three fields explicitly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/client/src/pages/Editor/HelpButton.tsx` around lines 225 - 227, The
effect currently calls bootPylon(user) but only depends on user?.email, so
updates to user.name or user.username won't re-run and Pylon can have stale
identity; change the dependency to either the whole user object (useEffect(...,
[user])) or explicitly include all identity fields (useEffect(..., [user?.name,
user?.username, user?.email])) so bootPylon(user) runs whenever any identity
field changes.
Comment on lines
+114
to
+115
| - Per-file max: 200 MB (widget) | ||
| - Total request max: 150 MB (server, configurable) |
Contributor
There was a problem hiding this comment.
Documented limits are internally inconsistent for end users.
Stating “Per-file max: 200 MB” alongside “Total request max: 150 MB” is misleading in a single-upload context, since a 200 MB file cannot pass a 150 MB request cap.
Suggested doc wording fix
- - Per-file max: 200 MB (widget)
- - Total request max: 150 MB (server, configurable)
+ - Widget per-file setting supports up to 200 MB, but effective upload size is still bounded by server multipart request limit.
+ - Total request max: 150 MB by default (server, configurable via `APPSMITH_CODEC_SIZE`).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - Per-file max: 200 MB (widget) | |
| - Total request max: 150 MB (server, configurable) | |
| - Widget per-file setting supports up to 200 MB, but effective upload size is still bounded by server multipart request limit. | |
| - Total request max: 150 MB by default (server, configurable via `APPSMITH_CODEC_SIZE`). |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/analysis/multi-file-upload-limits.md` around lines 114 - 115, The doc's
two limits contradict each other; update the wording in
multi-file-upload-limits.md to clarify that the widget supports a per-file
client-side limit of 200 MB but the default server-side total request cap is 150
MB (configurable), so uploads larger than the server cap will be rejected unless
the server config is changed; alternatively, align the numbers by changing the
per-file max to 150 MB if you intend the server cap to be the effective limit.
Make this change near the existing bullet points that currently read "Per-file
max: 200 MB (widget)" and "Total request max: 150 MB (server, configurable)" so
readers understand which limit applies and which one is enforced.
Comment on lines
+176
to
+191
| 2. **Reset workspaceRoles when workspace context changes** | ||
| - When `fetchRolesForWorkspace` is called with a new workspaceId, clear previous roles first to avoid showing stale data | ||
| - Or: store workspaceRoles per workspaceId in state | ||
|
|
||
| 3. **Ensure workspace is fully loaded before showing invite form** | ||
| - When opening share modal from app viewer, explicitly call `fetchWorkspace(application.workspaceId)` and `fetchRolesForWorkspace(application.workspaceId)` before rendering the form | ||
| - Show loading state until workspace and roles are loaded | ||
|
|
||
| 4. **Set full workspace object in app viewer init** | ||
| - When `SET_CURRENT_WORKSPACE_ID` is dispatched, also fetch and set the full workspace object so `currentWorkspace` has name and other fields | ||
| - Or: dispatch `FETCH_WORKSPACE` after `SET_CURRENT_WORKSPACE_ID` in the init flow | ||
|
|
||
| 5. **Clear workspaceRoles when workspace context changes** | ||
| - When `SET_CURRENT_WORKSPACE_ID` is dispatched with a different workspaceId, clear `state.ui.workspaces.workspaceRoles` (set to `[]`) | ||
| - Forces InviteUsersForm to show loading/empty until `fetchAllRoles(workspaceId)` completes with correct roles | ||
| - Prevents displaying roles from the wrong workspace |
Contributor
There was a problem hiding this comment.
Duplicate recommendation creates ambiguity.
Line 176–179 and Line 188–191 both recommend clearing/resetting workspaceRoles on workspace change. Keep one canonical recommendation to avoid confusion during implementation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/analysis/share-from-app-datasource-issues.md` around lines 176 - 191,
The document duplicates the same recommendation to clear/reset workspaceRoles on
workspace context change; remove one of the two entries so there is a single
canonical instruction. Specifically, keep the recommendation that best fits the
flow (e.g., the one tied to SET_CURRENT_WORKSPACE_ID and clearing
state.ui.workspaces.workspaceRoles to [] and triggering
fetchAllRoles(fetchRolesForWorkspace) for the InviteUsersForm), and delete the
other duplicate paragraph that references workspaceRoles/fetchRolesForWorkspace
so only one clear action is documented alongside related symbols like
SET_CURRENT_WORKSPACE_ID, fetchWorkspace, fetchRolesForWorkspace, fetchAllRoles,
workspaceRoles, currentWorkspace and InviteUsersForm.
| @@ -0,0 +1,555 @@ | |||
| # HTML Lang Attribute Customization — Implementation Plan | |||
|
|
|||
| > **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. | |||
Contributor
There was a problem hiding this comment.
Remove agent-specific execution directive from committed plan
Line 3 includes implementation instructions tied to a specific assistant/tooling persona. This is not product/engineering plan content and should be removed or rewritten as neutral contributor guidance.
Suggested edit
-> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.
+> Implementation note: Execute tasks in order and validate each phase before proceeding.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| > **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. | |
| > Implementation note: Execute tasks in order and validate each phase before proceeding. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/2026-03-05-html-lang-customization.md` at line 3, Remove the
agent-specific execution directive phrase "**For Claude:** REQUIRED SUB-SKILL:
Use superpowers:executing-plans to implement this plan task-by-task" from the
plan and replace it with neutral contributor guidance such as a general
instruction to "Implement this plan task-by-task" or "Follow the plan
step-by-step" so the document contains product/engineering content only; ensure
the edited sentence preserves intent but omits any assistant/tooling persona or
platform-specific "superpowers:executing-plans" reference.
Comment on lines
+392
to
+394
| ``` | ||
| app/client/src/ce/configs/index.ts | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add fenced code block language for markdownlint compliance
The fenced block at Line 392 is missing a language specifier (MD040).
Suggested edit
-```
+```text
app/client/src/ce/configs/index.ts🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 392-392: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/2026-03-05-html-lang-customization.md` around lines 392 - 394, The
fenced code block that currently contains the literal string
"app/client/src/ce/configs/index.ts" is missing a language specifier (MD040);
update that markdown block to use a language tag (e.g., change ``` to ```text)
so it reads ```text followed by app/client/src/ce/configs/index.ts and closing
``` to satisfy markdownlint and make the file path display correctly.
|
Deploy-Preview-URL: https://ce-41722.dp.appsmith.com |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/client/public/index.html (1)
234-285: Consider exposingdisablePylonin feature configs (optional).
DISABLE_PYLONis computed but not added toAPPSMITH_FEATURE_CONFIGS. Currently this works because when disabled, the SDK doesn't load, sowindow.Pylonwon't be a function andisPylonChatAvailable()returns false.If explicit disable-state awareness is ever needed in React code (e.g., for logging or conditional UI), you'd need to add
disablePylon: DISABLE_PYLON || falsehere and update the TypeScript interface.Not blocking—current behavior is functionally correct.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/client/public/index.html` around lines 234 - 285, APPSMITH_FEATURE_CONFIGS currently omits the computed DISABLE_PYLON flag, so React code cannot read explicit SDK-disable state; add a disablePylon: DISABLE_PYLON || false entry to window.APPSMITH_FEATURE_CONFIGS (next to pylonAppID) and update the corresponding TypeScript interface for feature configs to include disablePylon: boolean so code that calls isPylonChatAvailable() or needs explicit disable-state can read it safely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/client/public/index.html`:
- Around line 234-285: APPSMITH_FEATURE_CONFIGS currently omits the computed
DISABLE_PYLON flag, so React code cannot read explicit SDK-disable state; add a
disablePylon: DISABLE_PYLON || false entry to window.APPSMITH_FEATURE_CONFIGS
(next to pylonAppID) and update the corresponding TypeScript interface for
feature configs to include disablePylon: boolean so code that calls
isPylonChatAvailable() or needs explicit disable-state can read it safely.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e81466a9-4410-4635-89d4-01861b82f045
📒 Files selected for processing (1)
app/client/public/index.html
…refresh
- ContextualMenu: gate the CHAT_WIDGET option behind the same
cloudHosting || isIntercomConsentGiven check used by other Pylon
entry points so self-hosted users without consent never reach
window.Pylon. Pre-filter options and hoist getAppsmithConfigs() to
module scope.
- HelpButton: invoke updatePylonChatIdentity(instanceId, user) before
window.Pylon("show") in the returning/cloud-hosted user branch so
appsmith_version, instance_id, and license_id are pushed on every
open, matching the consent path.
- bootPylon.updatePylonChatIdentity: filter username through
ANONYMOUS_USERNAME, preserve the boot-time account_external_id (or
recompute it identically), and merge instead of replacing
window.pylon.chat_settings so consent refreshes don't drop the
original account linkage.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
app/client/src/components/editorComponents/Debugger/ContextualMenu.tsx (1)
131-151:⚠️ Potential issue | 🟡 MinorPrevent rendering a menu trigger when
visibleOptionsis empty.At Line 131 and Line 151,
getOptions()can return chat-only, and the filter can remove it; the trigger still renders with an empty popover.Suggested fix
const visibleOptions = getOptions( props.error.type, props.error.subType, ).filter( (e) => e !== CONTEXT_MENU_ACTIONS.CHAT_WIDGET || (isPylonChatAvailable() && isPylonConsentSatisfied), ); + + if (!visibleOptions.length) { + return props.children; + } return ( <Menu className="t--debugger-contextual-error-menu">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/client/src/components/editorComponents/Debugger/ContextualMenu.tsx` around lines 131 - 151, visibleOptions can be empty after filtering (getOptions(...).filter(...)), but the component still renders MenuTrigger/MenuContent causing an empty popover; change the render logic in ContextualMenu to first compute visibleOptions and then only render the Tooltip/MenuTrigger and MenuContent when visibleOptions.length > 0 (or return props.children directly / null when there are no visible options). Update use of visibleOptions, MenuTrigger, and MenuContent so the popover/trigger is not mounted if visibleOptions is empty, preserving existing checks for CONTEXT_MENU_ACTIONS.CHAT_WIDGET and pylon consent.app/client/src/pages/Editor/HelpButton.tsx (1)
226-228:⚠️ Potential issue | 🟡 MinorExpand the dependency array to cover all identity fields used by
bootPylon.
bootPylon()extractsname,username, and[user?.email]means changes tonameorusernamewon't trigger a re-run, leaving Pylon with stale identity data.Simplest fix: use
[user]as the dependency.Suggested fix
useEffect(() => { bootPylon(user); - }, [user?.email]); + }, [user]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/client/src/pages/Editor/HelpButton.tsx` around lines 226 - 228, The effect currently calls bootPylon(user) but only depends on user?.email, so changes to user.name or user.username won't re-run and Pylon may get stale identity; update the dependency array for the useEffect that calls bootPylon (the useEffect surrounding bootPylon(user)) to include the whole user object (or all identity fields used: name, username, email) so the effect re-runs whenever any of those identity values change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/client/src/components/editorComponents/Debugger/ContextualMenu.tsx`:
- Around line 131-151: visibleOptions can be empty after filtering
(getOptions(...).filter(...)), but the component still renders
MenuTrigger/MenuContent causing an empty popover; change the render logic in
ContextualMenu to first compute visibleOptions and then only render the
Tooltip/MenuTrigger and MenuContent when visibleOptions.length > 0 (or return
props.children directly / null when there are no visible options). Update use of
visibleOptions, MenuTrigger, and MenuContent so the popover/trigger is not
mounted if visibleOptions is empty, preserving existing checks for
CONTEXT_MENU_ACTIONS.CHAT_WIDGET and pylon consent.
In `@app/client/src/pages/Editor/HelpButton.tsx`:
- Around line 226-228: The effect currently calls bootPylon(user) but only
depends on user?.email, so changes to user.name or user.username won't re-run
and Pylon may get stale identity; update the dependency array for the useEffect
that calls bootPylon (the useEffect surrounding bootPylon(user)) to include the
whole user object (or all identity fields used: name, username, email) so the
effect re-runs whenever any of those identity values change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9230e8bf-dc0c-423e-9258-e06b460833ee
📒 Files selected for processing (3)
app/client/src/components/editorComponents/Debugger/ContextualMenu.tsxapp/client/src/pages/Editor/HelpButton.tsxapp/client/src/utils/bootPylon.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/src/utils/bootPylon.ts
Contributor
Author
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/24154776670. |
|
Deploy-Preview-URL: https://ce-41722.dp.appsmith.com |
Introduces APPSMITH_PYLON_IDENTITY_SECRET (hex) as a new server env var and plumbs it through Dockerfile, deploy_preview.sh, and the docker image CI workflows. The server computes HMAC-SHA256(secret, email) per /me response and the client sets it as window.pylon.chat_settings.email_hash so the Pylon widget boots with a verified identity. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/24167887008. |
|
Deploy-Preview-URL: https://ce-41722.dp.appsmith.com |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
.env.example (1)
55-57:⚠️ Potential issue | 🟡 MinorKeep the new Pylon env keys in linter order.
dotenv-linterwill still flag Line 57 here:APPSMITH_DISABLE_PYLONneeds to come beforeAPPSMITH_PYLON_APP_ID.Suggested fix
# Pylon in-app chat (optional runtime override; prefer REACT_APP_PYLON_APP_ID at build) -APPSMITH_PYLON_APP_ID= APPSMITH_DISABLE_PYLON= +APPSMITH_PYLON_APP_ID=🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.example around lines 55 - 57, The two Pylon env vars are out of linter-sorted order: APPSMITH_DISABLE_PYLON must appear before APPSMITH_PYLON_APP_ID to satisfy dotenv-linter; update the .env.example by reordering the entries so APPSMITH_DISABLE_PYLON comes immediately above APPSMITH_PYLON_APP_ID (preserve surrounding comments and formatting).
🧹 Nitpick comments (2)
app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/PylonIdentityHelperTest.java (1)
33-36: Add whitespace-only email case to matchhasTextsemantics.Since the helper treats whitespace as blank, adding
" "here would close a small contract gap.✅ Small test extension
`@Test` void computeEmailHash_returnsNullWhenEmailIsBlank() { assertThat(PylonIdentityHelper.computeEmailHash(VALID_SECRET_HEX, null)).isNull(); assertThat(PylonIdentityHelper.computeEmailHash(VALID_SECRET_HEX, "")).isNull(); + assertThat(PylonIdentityHelper.computeEmailHash(VALID_SECRET_HEX, " ")).isNull(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/PylonIdentityHelperTest.java` around lines 33 - 36, Update the test method computeEmailHash_returnsNullWhenEmailIsBlank to include a whitespace-only email case: call PylonIdentityHelper.computeEmailHash(VALID_SECRET_HEX, " ") and assert it returns null, matching the helper's hasText-style blank handling; this ensures computeEmailHash behavior for whitespace-only input is covered alongside the null and empty-string assertions.app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/PylonIdentityHelper.java (1)
27-38: Avoid exception-driven flow for invalid secret formats.If
secretHexis malformed, this path throws and logs a stack trace on every call. Consider a lightweight hex/length pre-check and early return to avoid repeated exception overhead and log spam.♻️ Proposed update
public static String computeEmailHash(String secretHex, String email) { if (!StringUtils.hasText(secretHex) || !StringUtils.hasText(email)) { return null; } + final String normalizedSecret = secretHex.trim(); + if ((normalizedSecret.length() % 2) != 0 || !normalizedSecret.matches("^[0-9a-fA-F]+$")) { + log.warn("Invalid Pylon identity secret format; skipping email hash generation"); + return null; + } try { Mac mac = Mac.getInstance("HmacSHA256"); - mac.init(new SecretKeySpec(Hex.decodeHex(secretHex), "HmacSHA256")); + mac.init(new SecretKeySpec(Hex.decodeHex(normalizedSecret), "HmacSHA256")); return Hex.encodeHexString(mac.doFinal(email.getBytes(StandardCharsets.UTF_8))); } catch (Exception e) { log.error("Failed to compute Pylon email hash", e); return null; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/PylonIdentityHelper.java` around lines 27 - 38, computeEmailHash currently relies on catching exceptions when secretHex is malformed, causing repeated stack traces; add a lightweight pre-check in computeEmailHash to validate secretHex is non-empty, has only hex characters and the expected length for a 32-byte HMAC key (hex length 64) and return null early if invalid, then proceed to decode and compute the HmacSHA256 only in the try/catch for unexpected runtime failures; reference computeEmailHash, Hex.decodeHex and SecretKeySpec/HmacSHA256 in your fix and remove/log only non-redundant errors to avoid exception-driven flow and log spam.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/client/src/utils/bootPylon.ts`:
- Around line 89-112: The current updatePylonChatIdentity flow sets
window.pylon.chat_settings.email_hash but never injects the actual Pylon SDK,
leaving only the queue/stubbed window.Pylon; modify updatePylonChatIdentity to
detect when there was no existing verified email_hash and a verified hash is now
present (compare existingEmailHash vs user?.emailVerificationHash) and call
injectPylonScriptOnce() right after setting window.pylon (or before invoking
window.Pylon("setNewIssueCustomFields")), ensuring the real SDK is loaded so
queued calls execute; use the same injectPylonScriptOnce function used by
bootPylon to avoid duplication.
In `@Dockerfile`:
- Around line 15-16: The Dockerfile is persisting the Pylon identity secret by
using ARG APPSMITH_PYLON_IDENTITY_SECRET followed by ENV
APPSMITH_PYLON_IDENTITY_SECRET, which bakes the HMAC key into the image
metadata; remove the ENV assignment (and avoid persisting the ARG into image
layers) and instead rely on injecting APPSMITH_PYLON_IDENTITY_SECRET at runtime
from a secret store (e.g., Kubernetes secret, Docker secret, or runtime-mounted
file) so the secret is never written into the image; update deployment
manifests/entrypoint to read the secret from the injected runtime env var or
file and reference APPSMITH_PYLON_IDENTITY_SECRET only at runtime.
In `@docs/plans/2026-04-08-pylon-identity-verification-design.md`:
- Line 17: The fenced code block labeled "architecture" is missing a language
specifier which triggers markdownlint MD040; update the fence for that block
(the triple-backtick fenced architecture block in the document) to include an
appropriate language tag (e.g., ```text or ```mermaid or ```yaml depending on
content) so the block has a language identifier.
- Line 24: The doc is out of sync with the implementation: update the design doc
snippets to reflect the actual static utility signature computeEmailHash(String
secretHex, String email) (and its call site using
commonConfig.getPylonIdentitySecret()) instead of showing a Spring `@Component`
with computeEmailHash(email); specifically replace occurrences like
PylonIdentityHelper.computeEmailHash(email) and the DI-based examples (lines
referenced around 24, 72–76, 95–100) with the real static call pattern and a
brief note about where to obtain the secret
(commonConfig.getPylonIdentitySecret()), or alternatively change the
implementation to a non-static component method if you prefer the DI design—pick
one approach and make all snippets consistent with the chosen implementation.
In `@scripts/deploy_preview.sh`:
- Line 116: The deployment currently places APPSMITH_PYLON_IDENTITY_SECRET into
applicationConfig.APPSMITH_PYLON_IDENTITY_SECRET which renders into a ConfigMap
(and becomes envFrom), exposing the HMAC secret; instead, stop setting
applicationConfig.APPSMITH_PYLON_IDENTITY_SECRET and move the value into a
Kubernetes Secret, then update the chart values to reference it via secretRef
(or the equivalent secretKeyRef) so the pod consumes
APPSMITH_PYLON_IDENTITY_SECRET from a Secret rather than the ConfigMap; adjust
any templates or the Helm values where applicationConfig is used to read from
the Secret and ensure envFrom is not used for this sensitive key.
- Line 33: The export of APPSMITH_PYLON_IDENTITY_SECRET currently references the
variable directly which triggers set -u when the variable is unset; change the
export to avoid failing under -u by either exporting only when
APPSMITH_PYLON_IDENTITY_SECRET is set or using parameter expansion to default it
to an empty string (so the export won't error when unset). Update the line
referencing APPSMITH_PYLON_IDENTITY_SECRET accordingly (or wrap it in a simple
if [ -n ... ] guard) so preview deploys don't abort when the Pylon secret is
intentionally omitted.
---
Duplicate comments:
In @.env.example:
- Around line 55-57: The two Pylon env vars are out of linter-sorted order:
APPSMITH_DISABLE_PYLON must appear before APPSMITH_PYLON_APP_ID to satisfy
dotenv-linter; update the .env.example by reordering the entries so
APPSMITH_DISABLE_PYLON comes immediately above APPSMITH_PYLON_APP_ID (preserve
surrounding comments and formatting).
---
Nitpick comments:
In
`@app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/PylonIdentityHelper.java`:
- Around line 27-38: computeEmailHash currently relies on catching exceptions
when secretHex is malformed, causing repeated stack traces; add a lightweight
pre-check in computeEmailHash to validate secretHex is non-empty, has only hex
characters and the expected length for a 32-byte HMAC key (hex length 64) and
return null early if invalid, then proceed to decode and compute the HmacSHA256
only in the try/catch for unexpected runtime failures; reference
computeEmailHash, Hex.decodeHex and SecretKeySpec/HmacSHA256 in your fix and
remove/log only non-redundant errors to avoid exception-driven flow and log
spam.
In
`@app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/PylonIdentityHelperTest.java`:
- Around line 33-36: Update the test method
computeEmailHash_returnsNullWhenEmailIsBlank to include a whitespace-only email
case: call PylonIdentityHelper.computeEmailHash(VALID_SECRET_HEX, " ") and
assert it returns null, matching the helper's hasText-style blank handling; this
ensures computeEmailHash behavior for whitespace-only input is covered alongside
the null and empty-string assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b37142eb-e4b0-44e8-9b73-04279266b767
📒 Files selected for processing (19)
.env.example.github/workflows/ad-hoc-docker-image.yml.github/workflows/client-build.yml.github/workflows/github-release.yml.github/workflows/on-demand-build-docker-image-deploy-preview.yml.github/workflows/test-build-docker-image.ymlDockerfileapp/client/public/index.htmlapp/client/src/ce/configs/index.tsapp/client/src/constants/userConstants.tsapp/client/src/utils/bootPylon.tsapp/server/appsmith-server/src/main/java/com/appsmith/server/configurations/CommonConfig.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ce/UserProfileCE_DTO.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/helpers/PylonIdentityHelper.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserServiceCEImpl.javaapp/server/appsmith-server/src/test/java/com/appsmith/server/helpers/PylonIdentityHelperTest.javaapp/server/envs/dev.env.exampledocs/plans/2026-04-08-pylon-identity-verification-design.mdscripts/deploy_preview.sh
✅ Files skipped from review due to trivial changes (4)
- app/client/src/constants/userConstants.ts
- app/server/envs/dev.env.example
- app/client/src/ce/configs/index.ts
- app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ce/UserProfileCE_DTO.java
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/github-release.yml
Comment on lines
+89
to
+112
| // Preserve any boot-time email_hash if the refreshed user object doesn't carry | ||
| // one (e.g. partial profile update), so identity verification survives across | ||
| // consent refreshes. | ||
| const existingEmailHash = window.pylon?.chat_settings?.email_hash; | ||
| const emailHash: string | undefined = | ||
| user?.emailVerificationHash || existingEmailHash; | ||
|
|
||
| // NOTE: account_external_id intentionally omitted. See bootPylon() above. | ||
| window.pylon = { | ||
| chat_settings: { | ||
| app_id: pylonAppID, | ||
| email, | ||
| name, | ||
| ...(emailHash ? { email_hash: emailHash } : {}), | ||
| }, | ||
| }; | ||
|
|
||
| window.Pylon("setNewIssueCustomFields", { | ||
| appsmith_version: `Appsmith ${ | ||
| !cloudHosting ? appVersion.edition : "" | ||
| } ${appVersion.id}`, | ||
| instance_id: instanceId, | ||
| license_id: getLicenseKey() ?? "", | ||
| }); |
Contributor
There was a problem hiding this comment.
Inject the SDK when updatePylonChatIdentity() gets the first verified hash.
This path preserves or sets email_hash, but unlike bootPylon() it never calls injectPylonScriptOnce(). On the consent flow in app/client/src/pages/Editor/HelpButton.tsx, that leaves only the stubbed window.Pylon queue in place, so the widget does not actually load until a refresh.
Suggested fix
const emailHash: string | undefined =
user?.emailVerificationHash || existingEmailHash;
// NOTE: account_external_id intentionally omitted. See bootPylon() above.
window.pylon = {
chat_settings: {
app_id: pylonAppID,
email,
name,
...(emailHash ? { email_hash: emailHash } : {}),
},
};
+
+ if (emailHash) {
+ injectPylonScriptOnce(pylonAppID);
+ }
window.Pylon("setNewIssueCustomFields", {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Preserve any boot-time email_hash if the refreshed user object doesn't carry | |
| // one (e.g. partial profile update), so identity verification survives across | |
| // consent refreshes. | |
| const existingEmailHash = window.pylon?.chat_settings?.email_hash; | |
| const emailHash: string | undefined = | |
| user?.emailVerificationHash || existingEmailHash; | |
| // NOTE: account_external_id intentionally omitted. See bootPylon() above. | |
| window.pylon = { | |
| chat_settings: { | |
| app_id: pylonAppID, | |
| email, | |
| name, | |
| ...(emailHash ? { email_hash: emailHash } : {}), | |
| }, | |
| }; | |
| window.Pylon("setNewIssueCustomFields", { | |
| appsmith_version: `Appsmith ${ | |
| !cloudHosting ? appVersion.edition : "" | |
| } ${appVersion.id}`, | |
| instance_id: instanceId, | |
| license_id: getLicenseKey() ?? "", | |
| }); | |
| // Preserve any boot-time email_hash if the refreshed user object doesn't carry | |
| // one (e.g. partial profile update), so identity verification survives across | |
| // consent refreshes. | |
| const existingEmailHash = window.pylon?.chat_settings?.email_hash; | |
| const emailHash: string | undefined = | |
| user?.emailVerificationHash || existingEmailHash; | |
| // NOTE: account_external_id intentionally omitted. See bootPylon() above. | |
| window.pylon = { | |
| chat_settings: { | |
| app_id: pylonAppID, | |
| email, | |
| name, | |
| ...(emailHash ? { email_hash: emailHash } : {}), | |
| }, | |
| }; | |
| if (emailHash) { | |
| injectPylonScriptOnce(pylonAppID); | |
| } | |
| window.Pylon("setNewIssueCustomFields", { | |
| appsmith_version: `Appsmith ${ | |
| !cloudHosting ? appVersion.edition : "" | |
| } ${appVersion.id}`, | |
| instance_id: instanceId, | |
| license_id: getLicenseKey() ?? "", | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/client/src/utils/bootPylon.ts` around lines 89 - 112, The current
updatePylonChatIdentity flow sets window.pylon.chat_settings.email_hash but
never injects the actual Pylon SDK, leaving only the queue/stubbed window.Pylon;
modify updatePylonChatIdentity to detect when there was no existing verified
email_hash and a verified hash is now present (compare existingEmailHash vs
user?.emailVerificationHash) and call injectPylonScriptOnce() right after
setting window.pylon (or before invoking
window.Pylon("setNewIssueCustomFields")), ensuring the real SDK is loaded so
queued calls execute; use the same injectPylonScriptOnce function used by
bootPylon to avoid duplication.
Comment on lines
+15
to
+16
| ARG APPSMITH_PYLON_IDENTITY_SECRET | ||
| ENV APPSMITH_PYLON_IDENTITY_SECRET=${APPSMITH_PYLON_IDENTITY_SECRET} |
Contributor
There was a problem hiding this comment.
Do not bake the Pylon identity secret into the image.
Passing APPSMITH_PYLON_IDENTITY_SECRET through ARG and then persisting it with ENV stores the HMAC key in image metadata. Anyone who can inspect or pull the image can recover it and mint valid email_hash values for arbitrary emails. This needs to be injected only at runtime from a secret store.
Suggested direction
-ARG APPSMITH_PYLON_IDENTITY_SECRET
-ENV APPSMITH_PYLON_IDENTITY_SECRET=${APPSMITH_PYLON_IDENTITY_SECRET}Then source it from the deployment environment via a runtime secret mount / secret-backed env var instead of the image build.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ARG APPSMITH_PYLON_IDENTITY_SECRET | |
| ENV APPSMITH_PYLON_IDENTITY_SECRET=${APPSMITH_PYLON_IDENTITY_SECRET} |
🧰 Tools
🪛 Trivy (0.69.3)
[error] 15-15: Secrets passed via build-args or envs or copied secret files
Possible exposure of secret env "APPSMITH_PYLON_IDENTITY_SECRET" in ARG
Rule: DS-0031
(IaC/Dockerfile)
[error] 16-16: Secrets passed via build-args or envs or copied secret files
Possible exposure of secret env "APPSMITH_PYLON_IDENTITY_SECRET" in ENV
Rule: DS-0031
(IaC/Dockerfile)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Dockerfile` around lines 15 - 16, The Dockerfile is persisting the Pylon
identity secret by using ARG APPSMITH_PYLON_IDENTITY_SECRET followed by ENV
APPSMITH_PYLON_IDENTITY_SECRET, which bakes the HMAC key into the image
metadata; remove the ENV assignment (and avoid persisting the ARG into image
layers) and instead rely on injecting APPSMITH_PYLON_IDENTITY_SECRET at runtime
from a secret store (e.g., Kubernetes secret, Docker secret, or runtime-mounted
file) so the secret is never written into the image; update deployment
manifests/entrypoint to read the secret from the injected runtime env var or
file and reference APPSMITH_PYLON_IDENTITY_SECRET only at runtime.
|
|
||
| ## Architecture | ||
|
|
||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language tag to the fenced architecture block.
This code fence is missing a language specifier (markdownlint MD040).
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 17-17: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/2026-04-08-pylon-identity-verification-design.md` at line 17, The
fenced code block labeled "architecture" is missing a language specifier which
triggers markdownlint MD040; update the fence for that block (the
triple-backtick fenced architecture block in the document) to include an
appropriate language tag (e.g., ```text or ```mermaid or ```yaml depending on
content) so the block has a language identifier.
| Spring CommonConfig.pylonIdentitySecret | ||
| │ | ||
| ▼ | ||
| PylonIdentityHelper.computeEmailHash(email) ─┐ |
Contributor
There was a problem hiding this comment.
Design doc API description is out of sync with the implemented helper.
These sections describe a Spring @Component and computeEmailHash(String email) with DI, but the implementation is a static utility method computeEmailHash(String secretHex, String email) called with commonConfig.getPylonIdentitySecret(). Please align the doc text/snippets to avoid future confusion.
Also applies to: 72-76, 95-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/2026-04-08-pylon-identity-verification-design.md` at line 24, The
doc is out of sync with the implementation: update the design doc snippets to
reflect the actual static utility signature computeEmailHash(String secretHex,
String email) (and its call site using commonConfig.getPylonIdentitySecret())
instead of showing a Spring `@Component` with computeEmailHash(email);
specifically replace occurrences like
PylonIdentityHelper.computeEmailHash(email) and the DI-based examples (lines
referenced around 24, 72–76, 95–100) with the real static call pattern and a
brief note about where to obtain the secret
(commonConfig.getPylonIdentitySecret()), or alternatively change the
implementation to a non-static component method if you prefer the DI design—pick
one approach and make all snippets consistent with the chosen implementation.
| --set applicationConfig.APPSMITH_CARBON_API_BASE_PATH="$APPSMITH_CARBON_API_BASE_PATH" \ | ||
| --set applicationConfig.APPSMITH_AI_SERVER_MANAGED_HOSTING="$APPSMITH_AI_SERVER_MANAGED_HOSTING" \ | ||
| --set applicationConfig.APPSMITH_BETTERBUGS_API_KEY="$APPSMITH_BETTERBUGS_API_KEY" \ | ||
| --set applicationConfig.APPSMITH_PYLON_IDENTITY_SECRET="$APPSMITH_PYLON_IDENTITY_SECRET" \ |
Contributor
There was a problem hiding this comment.
Do not ship the Pylon identity secret through applicationConfig.
In this chart, applicationConfig.* is rendered into the shared ConfigMap and then loaded with envFrom, so this line makes the HMAC secret readable to anyone with ConfigMap access in the preview namespace. Please move it to a Kubernetes Secret / secretRef path instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/deploy_preview.sh` at line 116, The deployment currently places
APPSMITH_PYLON_IDENTITY_SECRET into
applicationConfig.APPSMITH_PYLON_IDENTITY_SECRET which renders into a ConfigMap
(and becomes envFrom), exposing the HMAC secret; instead, stop setting
applicationConfig.APPSMITH_PYLON_IDENTITY_SECRET and move the value into a
Kubernetes Secret, then update the chart values to reference it via secretRef
(or the equivalent secretKeyRef) so the pod consumes
APPSMITH_PYLON_IDENTITY_SECRET from a Secret rather than the ConfigMap; adjust
any templates or the Helm values where applicationConfig is used to read from
the Secret and ensure envFrom is not used for this sensitive key.
Contributor
Author
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/24198519167. |
deploy_preview.sh runs under `set -u`, so the unconditional `export APPSMITH_PYLON_IDENTITY_SECRET="$APPSMITH_PYLON_IDENTITY_SECRET"` aborted the deploy-preview job whenever the variable wasn't provided by the calling workflow (e.g. repository_dispatch runs whose workflow YAML comes from `release`, which does not yet pass this env var through). Default to empty so previews can boot in unverified Pylon mode; the Java side (PylonIdentityHelper) already treats a blank secret as "no hash", so the chat widget gracefully degrades. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/24204600588. |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/24204615819. |
|
Deploy-Preview-URL: https://ce-41722.dp.appsmith.com |
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
1 participant
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.
Description
Move from Intercom integration for chat widget to Pylon integration
Warning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Caution
🔴 🔴 🔴 Some tests have failed.
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/24204553817
Commit: 106aa52
Cypress dashboard.
Tags: @tag.All
Spec:
The following are new failures, please fix them before merging the PR:
- cypress/e2e/Regression/ServerSide/QueryPane/DSDocs_Spec.ts
List of identified flaky tests.Thu, 09 Apr 2026 21:11:39 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Configuration Changes
Tests
Documentation