Skip to content

refactor: collapse TLS/Certificate section into accordion in MCP client form and sheet - #3854

Merged
Pratham-Mishra04 merged 1 commit into
devfrom
05-28-feat_use_accordion_for_tls_settings
May 28, 2026
Merged

Pratham-Mishra04 merged 1 commit into
devfrom
05-28-feat_use_accordion_for_tls_settings

Conversation

@BearTS

@BearTS BearTS commented May 28, 2026

Copy link
Copy Markdown
Contributor

Summary

The TLS / Certificate section in the MCP client form and sheet was always fully expanded, taking up visual space even when users don't need to configure TLS settings. This wraps the section in a collapsible Accordion component so it stays hidden by default and can be expanded on demand.

Additionally, the temporary-token SSO warning alert and its inline "Log in instead" button have been removed from the OAuth auth flow, and a minor punctuation fix was applied to the auth success page.

Changes

  • Replaced the static bordered div containing the TLS / Certificate fields with a collapsible Accordion in both mcpClientForm.tsx and mcpClientSheet.tsx
  • The accordion is collapsed by default, reducing visual clutter for users who don't need TLS configuration
  • The AccordionTrigger retains the "TLS / Certificate" label and includes a data-testid for test targeting
  • Removed the temporary-token SSO warning Alert block from the OAuth auth view in mcp-sessions/auth/page.tsx
  • Replaced an em dash with a hyphen in the auth success page copy

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

  1. Open the MCP client creation form or edit sheet for an HTTP or SSE connection type
  2. Verify the "TLS / Certificate" section is collapsed by default
  3. Click the accordion trigger to expand it and confirm the "Skip TLS verification" toggle and "CA Certificate (PEM)" textarea are visible and functional
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build

Breaking changes

  • Yes
  • No

Security considerations

No security implications. The underlying TLS fields and their behavior are unchanged. The removed temporary-token warning was UI-only and did not affect any auth logic.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Two MCP registry form files move TLS/Certificate settings into collapsible Accordion sections; auth pages receive a punctuation tweak and the temporary-token alert is removed. All TLS form fields, react-hook-form wiring, disabled logic, and data-testid attributes are preserved.

Changes

TLS Configuration Accordion Refactor

Layer / File(s) Summary
Add Accordion import & sheet accordion
ui/app/workspace/mcp-registry/views/mcpClientSheet.tsx
Adds Accordion imports and replaces the static "TLS / Certificate" bordered block with an AccordionItem/AccordionTrigger/AccordionContent structure while keeping the same tls_config.insecure_skip_verify and tls_config.ca_cert_pem fields, disabled logic, and data-testid attributes.
Form accordion wrapper
ui/app/workspace/mcp-registry/views/mcpClientForm.tsx
Replaces the standalone TLS/Certificate bordered container with an AccordionItem (value="tls-config") whose trigger shows "TLS / Certificate" and whose content renders the existing TLS form fields and react-hook-form bindings unchanged.

Auth page UI tweaks

Layer / File(s) Summary
Success message dash update
ui/app/workspace/mcp-sessions/auth-success/page.tsx
Updates inline success message punctuation between “your MCP client” and the following clause (em dash → hyphen).
Remove temporary token alert
ui/app/workspace/mcp-sessions/auth/page.tsx
Deletes the conditional “Temporary token in use” alert block and its “Log in instead” button so the OAuth view proceeds directly to the flow details list; inline button elsewhere still respects the flag.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • maximhq/bifrost#3783: Both PRs touch the same MCP HTTP/SSE TLS configuration keys (tls_config.insecure_skip_verify, tls_config.ca_cert_pem).

Suggested reviewers

  • danpiths
  • akshaydeo

Poem

🐰 I hopped through JSX, with nimble paws,
Folded TLS panels into tidy laws.
A dash made smaller, an alert unshown,
Accordions click where fields were known.
Cheers — the UI hums, neat as a comb!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description check ✅ Passed The PR description comprehensively covers all required sections from the template with clear explanations of changes, test instructions, and security considerations.
Title check ✅ Passed The title accurately describes the main change: converting the TLS/Certificate section into a collapsible Accordion in both the MCP client form and sheet, which is the primary refactoring throughout the PR.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 05-28-feat_use_accordion_for_tls_settings

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

BearTS commented May 28, 2026

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@BearTS BearTS changed the title feat: use accordion for tls settings refactor: collapse TLS/Certificate section into an Accordion in MCP client form and sheet May 28, 2026
@BearTS
BearTS marked this pull request as ready for review May 28, 2026 14:38
@greptile-apps

greptile-apps Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 4/5

The TLS accordion changes are safe; the unrelated deletion of the SSO warning in auth/page.tsx removes the only in-page route for SSO users to log in when using a temp-token link and should be reviewed before merging.

The accordion refactor itself is a clean, low-risk visual change. The concern is the undocumented removal of the SSO temp-token alert in auth/page.tsx: after the change, users arriving via a temporary link on an SSO-enabled instance see no warning about credential attribution and no "Log in instead" button, which is a regression from the previous behaviour.

ui/app/workspace/mcp-sessions/auth/page.tsx — the SSO warning block removal needs explicit confirmation.

Important Files Changed

Filename Overview
ui/app/workspace/mcp-registry/views/mcpClientForm.tsx TLS/Certificate section wrapped in a collapsible Accordion, consistent with the existing OAuth advanced section pattern; data-testid attributes preserved.
ui/app/workspace/mcp-registry/views/mcpClientSheet.tsx Same TLS accordion refactor as mcpClientForm.tsx; data-testid attributes preserved; previously flagged defaultValue issue (pre-filled TLS) not addressed in this PR.
ui/app/workspace/mcp-sessions/auth/page.tsx Removes the SSO temp-token warning alert, leaving SSO users with no in-page option to log in when using a temporary link — a UX regression undocumented in the PR description.
ui/app/workspace/mcp-sessions/auth-success/page.tsx Trivial copy change: em dash replaced with a hyphen in the success message.

Reviews (3): Last reviewed commit: "feat: use accordion for tls settings" | Re-trigger Greptile

Comment thread ui/app/workspace/mcp-registry/views/mcpClientSheet.tsx
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Actionable comments posted: 0

@BearTS
BearTS force-pushed the 05-28-feat_use_accordion_for_tls_settings branch from 6e23dea to c0d642a Compare May 28, 2026 14:44
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Actionable comments posted: 0

@BearTS
BearTS force-pushed the 05-28-feat_use_accordion_for_tls_settings branch from c0d642a to f595ffa Compare May 28, 2026 14:51
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Actionable comments posted: 0

@BearTS BearTS changed the title refactor: collapse TLS/Certificate section into an Accordion in MCP client form and sheet refactor: collapse TLS/Certificate section into accordion in MCP client form and sheet May 28, 2026

Pratham-Mishra04 commented May 28, 2026

Copy link
Copy Markdown
Collaborator

Merge activity

  • May 28, 3:25 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • May 28, 3:25 PM UTC: @Pratham-Mishra04 merged this pull request with Graphite.

@Pratham-Mishra04
Pratham-Mishra04 merged commit 777c8ae into dev May 28, 2026
15 checks passed
@Pratham-Mishra04
Pratham-Mishra04 deleted the 05-28-feat_use_accordion_for_tls_settings branch May 28, 2026 15:25
akshaydeo pushed a commit that referenced this pull request May 29, 2026
…nt form and sheet (#3854)

## Summary

The TLS / Certificate section in the MCP client form and sheet was always fully expanded, taking up visual space even when users don't need to configure TLS settings. This wraps the section in a collapsible `Accordion` component so it stays hidden by default and can be expanded on demand.

Additionally, the temporary-token SSO warning alert and its inline "Log in instead" button have been removed from the OAuth auth flow, and a minor punctuation fix was applied to the auth success page.

## Changes

- Replaced the static bordered `div` containing the TLS / Certificate fields with a collapsible `Accordion` in both `mcpClientForm.tsx` and `mcpClientSheet.tsx`
- The accordion is collapsed by default, reducing visual clutter for users who don't need TLS configuration
- The `AccordionTrigger` retains the "TLS / Certificate" label and includes a `data-testid` for test targeting
- Removed the temporary-token SSO warning `Alert` block from the OAuth auth view in `mcp-sessions/auth/page.tsx`
- Replaced an em dash with a hyphen in the auth success page copy

## Type of change

- [ ] Bug fix
- [ ] Feature
- [x] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (React)
- [ ] Docs

## How to test

1. Open the MCP client creation form or edit sheet for an HTTP or SSE connection type
2. Verify the "TLS / Certificate" section is collapsed by default
3. Click the accordion trigger to expand it and confirm the "Skip TLS verification" toggle and "CA Certificate (PEM)" textarea are visible and functional

```sh
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```

## Breaking changes

- [ ] Yes
- [x] No

## Security considerations

No security implications. The underlying TLS fields and their behavior are unchanged. The removed temporary-token warning was UI-only and did not affect any auth logic.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
akhsaul pushed a commit to akhsaul/bifrost that referenced this pull request Aug 27, 2026
…nt form and sheet (maximhq#3854)

## Summary

The TLS / Certificate section in the MCP client form and sheet was always fully expanded, taking up visual space even when users don't need to configure TLS settings. This wraps the section in a collapsible `Accordion` component so it stays hidden by default and can be expanded on demand.

Additionally, the temporary-token SSO warning alert and its inline "Log in instead" button have been removed from the OAuth auth flow, and a minor punctuation fix was applied to the auth success page.

## Changes

- Replaced the static bordered `div` containing the TLS / Certificate fields with a collapsible `Accordion` in both `mcpClientForm.tsx` and `mcpClientSheet.tsx`
- The accordion is collapsed by default, reducing visual clutter for users who don't need TLS configuration
- The `AccordionTrigger` retains the "TLS / Certificate" label and includes a `data-testid` for test targeting
- Removed the temporary-token SSO warning `Alert` block from the OAuth auth view in `mcp-sessions/auth/page.tsx`
- Replaced an em dash with a hyphen in the auth success page copy

## Type of change

- [ ] Bug fix
- [ ] Feature
- [x] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (React)
- [ ] Docs

## How to test

1. Open the MCP client creation form or edit sheet for an HTTP or SSE connection type
2. Verify the "TLS / Certificate" section is collapsed by default
3. Click the accordion trigger to expand it and confirm the "Skip TLS verification" toggle and "CA Certificate (PEM)" textarea are visible and functional

```sh
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```

## Breaking changes

- [ ] Yes
- [x] No

## Security considerations

No security implications. The underlying TLS fields and their behavior are unchanged. The removed temporary-token warning was UI-only and did not affect any auth logic.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
occcat pushed a commit to occcat/bifrost that referenced this pull request Sep 2, 2026
…nt form and sheet (maximhq#3854)

## Summary

The TLS / Certificate section in the MCP client form and sheet was always fully expanded, taking up visual space even when users don't need to configure TLS settings. This wraps the section in a collapsible `Accordion` component so it stays hidden by default and can be expanded on demand.

Additionally, the temporary-token SSO warning alert and its inline "Log in instead" button have been removed from the OAuth auth flow, and a minor punctuation fix was applied to the auth success page.

## Changes

- Replaced the static bordered `div` containing the TLS / Certificate fields with a collapsible `Accordion` in both `mcpClientForm.tsx` and `mcpClientSheet.tsx`
- The accordion is collapsed by default, reducing visual clutter for users who don't need TLS configuration
- The `AccordionTrigger` retains the "TLS / Certificate" label and includes a `data-testid` for test targeting
- Removed the temporary-token SSO warning `Alert` block from the OAuth auth view in `mcp-sessions/auth/page.tsx`
- Replaced an em dash with a hyphen in the auth success page copy

## Type of change

- [ ] Bug fix
- [ ] Feature
- [x] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (React)
- [ ] Docs

## How to test

1. Open the MCP client creation form or edit sheet for an HTTP or SSE connection type
2. Verify the "TLS / Certificate" section is collapsed by default
3. Click the accordion trigger to expand it and confirm the "Skip TLS verification" toggle and "CA Certificate (PEM)" textarea are visible and functional

```sh
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```

## Breaking changes

- [ ] Yes
- [x] No

## Security considerations

No security implications. The underlying TLS fields and their behavior are unchanged. The removed temporary-token warning was UI-only and did not affect any auth logic.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants