Skip to content

chore: Removed mcp server url settings from admin settings - #42208

Merged
sondermanish merged 3 commits into
releasefrom
chore/mcp-settings
Sep 8, 2026
Merged

sondermanish merged 3 commits into
releasefrom
chore/mcp-settings

Conversation

@sondermanish

@sondermanish sondermanish commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Description

  • Removed peripheral settings from MCP settings
  • Now mcp toggle will require restart on single org instances

Fixes https://linear.app/appsmith/issue/APP-15928/mcp-settings-toggle-should-require-a-restart-for-mcp-server-to-source

Automation

/ok-to-test tags="@tag.All"

Testing

Note

How CI runs on fork PRs — no action needed from you.

  1. Workflow approval. GitHub holds the first run on fork PRs until a maintainer approves it, so a pause before any check appears is expected.
  2. Credential-free checks. Once approved, format, lint, typecheck, unit tests, cyclic-dependency and compile-only build checks run without repository secrets. Only the checks relevant to what you changed (client / server / RTS) will run, and their logs are safe to debug against.
  3. Maintainer-triggered checks. Cypress, Playwright, Docker builds and deploy previews need secrets, so a maintainer starts them with /approve-ci, and /build-deploy-preview when hands-on testing is needed. Approval is pinned to one commit — pushing again requires fresh approval.

The awaiting-maintainer / awaiting-contributor labels show whose turn it is. You do not need ok-to-test or any slash command. Full detail: Pull request check states.

Select the validation relevant to this change:

  • Client unit tests
  • Server unit tests
  • Cypress
  • Playwright
  • Deploy preview
  • Not applicable

Suggested Cypress tags or specs:

Communication

Should the DevRel and Marketing teams inform users about this change?

  • Yes
  • No

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/34195617186
Commit: 584a55c
Cypress dashboard.
Tags: @tag.All
Spec:


Tue, 08 Sep 2026 07:42:19 UTC

Summary by CodeRabbit

  • New Features

    • MCP administration now exposes a single setting to enable or disable MCP.
    • Changes requiring a server restart are clearly identified, and the Save and Restart action remains available.
    • Organization configuration updates can automatically start restart-status polling.
  • Bug Fixes

    • Improved handling of MCP configuration updates that require a restart.
    • MCP internal secret changes no longer trigger an immediate server restart; they are applied through the Save and Restart flow.
  • Tests

    • Updated coverage for simplified MCP settings and restart behavior.

@sondermanish
sondermanish requested a review from a team as a code owner September 8, 2026 06:39
@linear-code

linear-code Bot commented Sep 8, 2026

Copy link
Copy Markdown

APP-15928

@sondermanish sondermanish self-assigned this Sep 8, 2026
@sondermanish sondermanish added the ok-to-test Required label for CI label Sep 8, 2026
@github-actions github-actions Bot added the skip-changelog Adding this label to a PR prevents it from being listed in the changelog label Sep 8, 2026
@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 97deffe0-c340-4641-aa40-db69cc2735e2

📥 Commits

Reviewing files that changed from the base of the PR and between 81da26b and 584a55c.

📒 Files selected for processing (17)
  • app/client/src/ce/api/OrganizationApi.ts
  • app/client/src/ce/pages/AdminSettings/config/mcpServer.ts
  • app/client/src/ce/pages/AdminSettings/config/types.ts
  • app/client/src/ce/sagas/organizationSagas.tsx
  • app/client/src/ce/utils/adminSettingsHelpers.test.ts
  • app/client/src/ce/utils/adminSettingsHelpers.ts
  • app/client/src/pages/AdminSettings/SaveSettings.tsx
  • app/client/src/pages/AdminSettings/SettingsForm.tsx
  • app/client/src/pages/AdminSettings/config/ConfigFactory.ts
  • app/server/appsmith-server/src/main/java/com/appsmith/server/domains/McpConfig.java
  • app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/McpOrganizationConfigurationHelper.java
  • app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/OrganizationServiceCEImpl.java
  • app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCE.java
  • app/server/appsmith-server/src/test/java/com/appsmith/server/domains/McpConfigTest.java
  • app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ce/McpOrganizationConfigurationHelperTest.java
  • app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/OrganizationServiceCETest.java
  • app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java
💤 Files with no reviewable changes (1)
  • app/client/src/ce/utils/adminSettingsHelpers.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.


Walkthrough

The change removes MCP data and server URL settings, keeps enabled as the MCP policy switch, and adds restart handling to organization settings updates. Client and server tests now cover the reduced configuration and secret persistence behavior.

Changes

MCP configuration and restart flow

Layer / File(s) Summary
MCP policy shape and settings reduction
app/client/src/ce/pages/AdminSettings/config/*, app/client/src/ce/utils/*, app/server/appsmith-server/src/main/java/com/appsmith/server/domains/McpConfig.java, app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/*, app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/*
MCP configuration now exposes only enabled. Client flattening, server copying, and organization updates no longer handle dataEnabled or serverUrl.
Restart requirement propagation
app/client/src/ce/api/OrganizationApi.ts, app/client/src/ce/pages/AdminSettings/config/*, app/client/src/pages/AdminSettings/*, app/client/src/ce/sagas/organizationSagas.tsx
The needsRestart flag moves from admin configuration through the settings form and save flow. Successful updates dispatch RESTART_SERVER_POLL when the flag is true.
Policy behavior and secret persistence validation
app/server/appsmith-server/src/test/java/com/appsmith/server/domains/*, app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ce/*, app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/*, app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/*, app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCE.java
Tests cover enabled fallback, removal of the previous MCP fields, preserved internal secrets, and secret persistence without an immediate restart.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to 584a5

MCP administration is reduced to the enabled switch, with single-organization changes using the Save and Restart flow. The updated configuration and persistence paths are covered without an identified merge-blocking risk.

Sequence Diagram(s)

sequenceDiagram
  participant AdminSettings
  participant OrganizationService
  participant updateOrganizationConfigSaga
  participant ServerRestartPoll
  AdminSettings->>OrganizationService: update MCP configuration with needsRestart
  OrganizationService-->>updateOrganizationConfigSaga: return successful update
  updateOrganizationConfigSaga->>ServerRestartPoll: dispatch RESTART_SERVER_POLL
  ServerRestartPoll-->>AdminSettings: track server restart
Loading

Suggested reviewers: subrata71, wyattwalter

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.88% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 16 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the removal of MCP server URL settings, which is a primary change in the pull request. It does not mention the restart requirement, but it remains specific and relevant.
Description check ✅ Passed The description explains the main changes, provides the related issue link, and includes the required Testing and Communication sections. The validation and communication checkboxes are not selected, …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/mcp-settings

One MCP switch stands bright and clear
Restart signals travel near
Settings save, the poll begins
Old fields fade; the new flow wins
Secrets rest without a start

Comment @coderabbitai help to get the list of available commands.

@sondermanish
sondermanish merged commit 9755a46 into release Sep 8, 2026
100 checks passed
@sondermanish
sondermanish deleted the chore/mcp-settings branch September 8, 2026 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Required label for CI skip-changelog Adding this label to a PR prevents it from being listed in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants