Skip to content

fix(lightspeed-mcp): refactored mcp config modal#3698

Open
ciiay wants to merge 12 commits into
redhat-developer:mainfrom
ciiay:rhidp-14843-mcp-config-modal-refactor
Open

fix(lightspeed-mcp): refactored mcp config modal#3698
ciiay wants to merge 12 commits into
redhat-developer:mainfrom
ciiay:rhidp-14843-mcp-config-modal-refactor

Conversation

@ciiay

@ciiay ciiay commented Jul 7, 2026

Copy link
Copy Markdown
Member

Hey, I just made a Pull Request!

For RHIDP-14843

Refactor the MCP server configure modal with PatternFly layout and improved credential management:

  • Add Use organization default token / Use personal token radio selectors when an app-config default token is available, replacing the remove-personal-token flow
  • Show server status, tools list, and enabled toggle with local draft state applied on Save
  • Hide the tools section when the server is disabled; keep status consistent while drafting an unverified personal token
  • Disable Save after a failed token validation until the input changes
  • Auto-enable the server after a successful personal token save
  • Add sortable Name and Status columns to the MCP servers table, with always-visible sort icons (gray when inactive, directional when active)
  • Keep the DCR configure modal as an information-only view (DCR explanation + Cancel) with no token or status/enabled controls
  • Bump @patternfly/react-core to 6.6.0 and add translations for new modal strings (de, es, fr, it, ja)

The backend now exposes hasOrgToken on MCP server list and patch responses so the UI can distinguish organization default tokens from personal-only servers.

Behavior confirmation: Intelligent-assistant MCP config modal details alignment

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

Test steps:

Test steps for Intelligent-assistant plugin

screen recording(updated on 7/13)

rhidp_14834.mp4

@rhdh-gh-app

rhdh-gh-app Bot commented Jul 7, 2026

Copy link
Copy Markdown

Important

This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior.

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/backstage-plugin-lightspeed-backend workspaces/lightspeed/plugins/lightspeed-backend patch v2.9.1
@red-hat-developer-hub/backstage-plugin-lightspeed workspaces/lightspeed/plugins/lightspeed minor v2.9.1

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.16327% with 66 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.92%. Comparing base (4361e77) to head (2b5f861).
⚠️ Report is 12 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3698      +/-   ##
==========================================
+ Coverage   54.59%   54.92%   +0.33%     
==========================================
  Files        2361     2362       +1     
  Lines       90132    90451     +319     
  Branches    25214    25342     +128     
==========================================
+ Hits        49205    49680     +475     
+ Misses      40645    40487     -158     
- Partials      282      284       +2     
Flag Coverage Δ *Carryforward flag
adoption-insights 83.70% <ø> (ø) Carriedforward from dcffc71
ai-integrations 67.53% <ø> (ø) Carriedforward from dcffc71
app-defaults 69.79% <ø> (ø) Carriedforward from dcffc71
augment 46.39% <ø> (ø) Carriedforward from dcffc71
boost 73.07% <ø> (ø) Carriedforward from dcffc71
bulk-import 72.46% <ø> (ø) Carriedforward from dcffc71
cost-management 14.10% <ø> (ø) Carriedforward from dcffc71
dcm 61.81% <ø> (ø) Carriedforward from dcffc71
extensions 61.53% <ø> (ø) Carriedforward from dcffc71
global-floating-action-button 71.18% <ø> (ø) Carriedforward from dcffc71
global-header 59.71% <ø> (ø) Carriedforward from dcffc71
homepage 50.30% <ø> (ø) Carriedforward from dcffc71
install-dynamic-plugins 56.77% <ø> (ø) Carriedforward from dcffc71
konflux 91.49% <ø> (ø) Carriedforward from dcffc71
lightspeed 73.37% <83.16%> (+4.31%) ⬆️
mcp-integrations 85.46% <ø> (ø) Carriedforward from dcffc71
orchestrator 42.97% <ø> (ø) Carriedforward from dcffc71
quickstart 65.63% <ø> (ø) Carriedforward from dcffc71
sandbox 79.56% <ø> (ø) Carriedforward from dcffc71
scorecard 82.67% <ø> (ø) Carriedforward from dcffc71
theme 61.26% <ø> (ø) Carriedforward from dcffc71
translations 7.25% <ø> (ø) Carriedforward from dcffc71
x2a 78.68% <ø> (ø) Carriedforward from dcffc71

*This pull request uses carry forward flags. Click here to find out more.


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4361e77...2b5f861. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@HusneShabbir HusneShabbir 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.

Hey @ciiay , I have a couple of questions:

  1. Why haven't we implemented the "forget token" action? Without it, disconnecting an MCP feels a bit bizarre. I also noticed it's explicitly mentioned as the third point in the RHIDP-14843 description.

  2. Would it be possible to eliminate the UI flicker that's happening when toggling the MCP on/off?

  3. I also noticed that when the toggle is turned off, the status shown in the panel doesn't change. That doesn't seem to match the behavior shown in the prototype.

Also, from a UX perspective, I think the Save button should become enabled whenever the user changes the toggle or token. Right now, the changes seem to take effect immediately, leaving Cancel as the only meaningful action, which was a bit confusing to me as a first-time user.

cc: @ShiranHi

Screen.Recording.2026-07-08.at.5.09.34.PM.mov

@ciiay ciiay force-pushed the rhidp-14843-mcp-config-modal-refactor branch from 2e456d9 to 6c03c52 Compare July 8, 2026 18:41
@ciiay ciiay mentioned this pull request Jul 8, 2026
4 tasks
@ciiay

ciiay commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

Hi @HusneShabbir , thanks for the review. Here's some background from previous discussion.

  • Button Renaming: Rename the “Forget token” button to “Remove personal token,” as tokens defined in the app-config cannot be removed or forgotten.
  • Button Visibility: Display the “Remove personal token” button only when a personal token has been provided by the user.
  • Token Priority & Fallback: User-provided tokens override the default app-config token. If a user removes their personal token for a server that has a default app-config token, the system will revert to connecting using that preserved default token.
  1. Why haven't we implemented the "forget token" action? Without it, disconnecting an MCP feels a bit bizarre. I also noticed it's explicitly mentioned as the third point in the RHIDP-14843 description.

It's implemented as "Remove personal token" and hidden for MCP servers that doesn't have a personal token input yet.

  1. Would it be possible to eliminate the UI flicker that's happening when toggling the MCP on/off?
  2. I also noticed that when the toggle is turned off, the status shown in the panel doesn't change. That doesn't seem to match the behavior shown in the prototype.

Good catch, fixed in my new commit.

Also, from a UX perspective, I think the Save button should become enabled whenever the user changes the toggle or token. Right now, the changes seem to take effect immediately, leaving Cancel as the only meaningful action, which was a bit confusing to me as a first-time user.

Addressed in my new commit 🤝

@HusneShabbir HusneShabbir 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.

Tested all the edge cases locally.
/lgtm

FYI: I'll cover the E2E tests for this implementation in RHIDP-15136.

@openshift-ci

openshift-ci Bot commented Jul 10, 2026

Copy link
Copy Markdown

New changes are detected. LGTM label has been removed.

@openshift-ci openshift-ci Bot removed the lgtm label Jul 10, 2026
ciiay added 9 commits July 9, 2026 23:18
Signed-off-by: Yi Cai <yicai@redhat.com>
Signed-off-by: Yi Cai <yicai@redhat.com>
Signed-off-by: Yi Cai <yicai@redhat.com>
Signed-off-by: Yi Cai <yicai@redhat.com>
Signed-off-by: Yi Cai <yicai@redhat.com>
Signed-off-by: Yi Cai <yicai@redhat.com>
Signed-off-by: Yi Cai <yicai@redhat.com>
Signed-off-by: Yi Cai <yicai@redhat.com>
Signed-off-by: Yi Cai <yicai@redhat.com>
@ciiay ciiay force-pushed the rhidp-14843-mcp-config-modal-refactor branch from a253f7c to 7417c68 Compare July 10, 2026 03:21
Signed-off-by: Yi Cai <yicai@redhat.com>
@HusneShabbir HusneShabbir self-requested a review July 10, 2026 06:30
@its-mitesh-kumar its-mitesh-kumar dismissed HusneShabbir’s stale review July 10, 2026 11:57

Instead of approve he he selected request changes,

@its-mitesh-kumar its-mitesh-kumar enabled auto-merge (squash) July 10, 2026 11:57
Signed-off-by: Yi Cai <yicai@redhat.com>

@HusneShabbir HusneShabbir 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.

Everything looks good to me. Thanks for addressing my review feedback!

I do have one observation, though: shouldn't the status of the DCR-authenticated MCP server match exactly what is shown in the sidebar?

Why does it show "Token required" even when the server is connected?

Image
Screen.Recording.2026-07-13.at.10.55.30.PM.mov

Signed-off-by: Yi Cai <yicai@redhat.com>
@ciiay

ciiay commented Jul 13, 2026

Copy link
Copy Markdown
Member Author

Hi @HusneShabbir , thanks for the review.

Why does it show "Token required" even when the server is connected?

This is the DCR auth flow that was not covered in my previous code changes. I have added fix for this flow in my latest commit. Can you take another look?

@sonarqubecloud

Copy link
Copy Markdown

@ciiay ciiay requested a review from HusneShabbir July 14, 2026 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants