feat(integrations): split MCP servers into own page#2765
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
daryllimyt
added
ui
|
✅ No security or compliance issues detected. Reviewed everything up to 00c7402. Security Overview
Detected Code ChangesThe diff is too large to display a summary of code changes. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ac090110c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 6 files
Confidence score: 3/5
- There is some merge risk because multiple permission-gating issues are concrete and user-facing (mostly severity 5–6 with high confidence), rather than just cosmetic or housekeeping concerns.
- Most severe: in
frontend/src/app/workspaces/[workspaceId]/mcp-servers/page.tsx, the create MCP dialog can be opened from URL params withoutintegration:update, which can expose mutation UI to read-only users. frontend/src/app/workspaces/[workspaceId]/mcp-servers/page.tsxalso conflates permission-loading (undefined) with denial, creating a false “Access denied” state; plusfrontend/src/components/sidebar/app-sidebar.tsxcan routesecret:readusers to/integrationswhere they are blocked.- Pay close attention to
frontend/src/app/workspaces/[workspaceId]/mcp-servers/page.tsx,frontend/src/components/sidebar/app-sidebar.tsx, andfrontend/src/components/nav/controls-header.tsx- permission checks and action visibility need to align with read vs create/update scopes.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/components/nav/controls-header.tsx">
<violation number="1" location="frontend/src/components/nav/controls-header.tsx:368">
P2: Gate the new “Key-value credentials” action by a create-secret scope. Right now read-only users can see and trigger a create flow they are not authorized to complete.</violation>
<violation number="2" location="frontend/src/components/nav/controls-header.tsx:2006">
P2: Adding `/settings/secrets` to `CredentialsActions` exposes an ungated create button on a read-visible page. Add a `secret:create` check before rendering this action.</violation>
</file>
<file name="frontend/src/app/workspaces/[workspaceId]/mcp-servers/page.tsx">
<violation number="1" location="frontend/src/app/workspaces/[workspaceId]/mcp-servers/page.tsx:93">
P2: Opening the create MCP dialog from URL params is not scoped to `integration:update`, exposing a mutation UI to read-only users.</violation>
<violation number="2" location="frontend/src/app/workspaces/[workspaceId]/mcp-servers/page.tsx:152">
P2: Treating `undefined` (permission-loading) as denial causes a false “Access denied” state. Handle loading and denied states separately.</violation>
</file>
<file name="frontend/src/components/sidebar/app-sidebar.tsx">
<violation number="1" location="frontend/src/components/sidebar/app-sidebar.tsx:188">
P2: Users with `secret:read` but without `integration:read` will see this sidebar item (due to the `canViewSecrets` visibility condition) but the URL always points to `/integrations`, which denies access without `integration:read`. This effectively removes sidebar navigation to the credentials inventory for secret-only roles. Consider conditionally routing to `/settings/secrets` when only `canViewSecrets` is true, or restoring a separate credentials nav item.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
daryllimyt
force-pushed
the
feat/integrations-ia-split
branch
from
9ac0901 to
b3fead8
Compare
daryllimyt
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3fead895f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c990add225
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c42ffbcc99
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
2 issues found across 10 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b4bbc044f4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae76ffae63
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ba1d95612
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Add an optional `source=platform|workspace` query param to GET /mcp-integrations. The MCP servers page now requests `source=workspace` so platform-managed rows (auto-created by MCPAuthProvider OAuth flows) no longer surface under the Workspace-authored section. Other callers (agent presets, runtime resolution) keep the unfiltered default. The filter resolves the linked OAuth integration's provider class against the registry rather than matching on the `_mcp` provider-id suffix, so future MCPAuthProvider additions don't need to follow the naming convention to be classified correctly.
Restyle the MCP servers page closer to the Langdock layout: - Drop the subtitle paragraph and the platform/workspace section headers; render one alphabetized 4-up grid mixing platform-shipped providers and workspace-authored MCP integrations. - Add single-select preset pill filters (All, Connected, Workspace) via the existing CatalogHeader pill mechanism; make the pill icon optional so the row can be icon-less. - Tighten card visuals: smaller description text, outline Connect button, no button icons, drop the auth_type badge, drop the stdio command from the card body, drop the inline Trash button. - Move the workspace MCP delete action into the edit dialog footer with a destructive confirm step; remove the page-level AlertDialog and pendingDelete state. Rework the OAuth integration details dialog into a connection manager: - Section the dialog into Connection / Configuration / Scopes with hairline separators and pull the description from provider metadata. - Surface the connection as a row with a status avatar, the provider name, an Expired badge when the token is past expiry, and a relative "Last updated" timestamp. - Replace the standalone Disconnect button with a kebab menu exposing Reauthorize (authorization_code only), Test, and Disconnect; Disconnect keeps the type-the-provider-name confirm. - Reserve dialog height and use layout-matching Skeleton placeholders so opening the dialog no longer flashes a centered spinner before jumping to full height.
Pull the duplicated pieces out of the integration surfaces so the page
files and the OAuth details dialog stop redefining the same shapes:
- frontend/src/lib/integrations.ts: isMcpProvider predicate and an
integrationKeys query-key factory. Replaces three inline
id.endsWith("_mcp") checks across the page and dialog files.
- frontend/src/lib/time.ts: formatRelative helper that returns null on
missing or invalid dates. Replaces inline IIFEs on the MCP servers
page and OAuth details dialog.
- frontend/src/hooks/use-integration-actions.ts: useConnectProvider,
useDisconnectProvider, useTestProvider hooks with shared cache
invalidation. Replaces three inline useMutation copies (integrations
page, OAuth details dialog, MCP servers page).
- frontend/src/components/confirm-destructive-dialog.tsx: reusable
type-the-name confirmation. Replaces two inline AlertDialog copies
(per-row disconnect on the integrations page, disconnect inside the
OAuth details dialog).
- frontend/src/components/integrations/mcp-integration-schema.ts:
zod schema, defaults, SERVER_TYPES, AUTH_TYPES, ALLOWED_COMMANDS,
and the isValidStringMap helper, moved out of the 1100-line dialog
component.
Call sites were updated to consume the shared modules. Net effect is
~525 fewer lines across the four edited files, and invalidation /
disconnect semantics are now defined in one place each.
Findings from a Vercel React best-practices pass:
- mcp-servers/page.tsx: conditionally mount the three dialogs
(OAuthIntegrationDetailsDialog and the two MCPIntegrationDialog
instances) so closed dialogs don't create mutation hooks,
AlertDialog portals, or subtree state. Matches the integrations
page pattern and frontend/CLAUDE.md ("lazily mount the data-owning
content only when open === true").
- mcp-servers/page.tsx: split filteredItems into two useMemos. The
preset filter only recomputes when items or the preset selection
change; the search filter only runs when the query changes. Typing
in search no longer reruns the preset pass and vice versa.
- mcp-servers/page.tsx: watch a primitive createSignal derived from
the URL instead of subscribing to the whole searchParams object
inside the create-signal effect. Inline the now-trivial signal
handler.
- confirm-destructive-dialog.tsx: drop the useEffect that reset
confirmText on close. The reset now happens inside an onOpenChange
wrapper, removing the extra render on close.
- integrations/page.tsx: hoist getIntegrationStatus and
getIntegrationDisplayType to module-level pure functions. Drop the
useCallback wrapper around handleTypeFilterToggle and remove the
now-stale entries from the dependent useMemo dep arrays.
- integrations/page.tsx: hoist searchQuery.toLowerCase() once outside
the filteredIntegrations loop instead of recomputing per item.
daryllimyt
force-pushed
the
feat/integrations-ia-split
branch
from
8ba1d95 to
cc93322
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 263e5a00e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a4042120c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de0e92b2ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tracecat/integrations/service.py">
<violation number="1" location="tracecat/integrations/service.py:1162">
P2: This deletion path only removes one matching MCP integration even when multiple candidates exist, which can leave stale provider-managed rows and preset references behind.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 731f5d8f92
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02466f49d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c04a034bea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…split # Conflicts: # scripts/tests/test_action_doc_links.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 292b39a1c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
2 participants
Summary
Screenshots
Sidebar and navigation
Integrations add menu
MCP servers grid
MCP provider configuration
OAuth details actions
Tests
uv run ruff check .pnpm -C frontend check