Skip to content

Comments

frontend: remove ADP related flags already enabled#2238

Merged
malinskibeniamin merged 6 commits intomasterfrom
feature-flag-cleanup
Feb 23, 2026
Merged

frontend: remove ADP related flags already enabled#2238
malinskibeniamin merged 6 commits intomasterfrom
feature-flag-cleanup

Conversation

@malinskibeniamin
Copy link
Contributor

@malinskibeniamin malinskibeniamin commented Feb 20, 2026

Also removes shadow link feature flag since it was already enabled across all environments for 2+ months.

Fix for UX-901

malinskibeniamin and others added 3 commits February 20, 2026 20:44
Add GetQuotas to the early-return false switch in isSupported() so it
doesn't flash in the sidebar during MFE init when endpointCompatibility
is still null.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@malinskibeniamin
Copy link
Contributor Author

@claude review

@claude
Copy link

claude bot commented Feb 20, 2026

Claude finished @malinskibeniamin's task in 2m 47s —— View job


Code Review

  • Gather context and understand the PR
  • Read key changed files
  • Analyze changes for correctness, issues, and quality
  • Post review feedback

Overall this is a clean flag-removal PR. The bulk of the code change is the API version consolidation in remote-mcp.tsx (removing the v1/v1alpha3 dual-dispatch pattern), which is the right move now that the enableMcpServiceAccount flag has been enabled everywhere. Tests are updated consistently throughout.

A few things to flag:


Bug: Wrong type parameter in useListMCPServersQuery

frontend/src/react-query/api/remote-mcp.tsx:44

// Current (wrong)
options?: QueryOptions<GenMessage<ListMCPServersRequest>, ListMCPServersRequest>

// Should be
options?: QueryOptions<GenMessage<ListMCPServersRequest>, ListMCPServersResponse>

ListMCPServersResponse is not imported in the new code, so ListMCPServersRequest was used as the second type parameter by mistake. The second generic on QueryOptions is SelectOutData — the expected output shape. Using the request type there is incorrect. It won't cause a runtime error (only options.enabled is consumed), but it's a type lie that could mislead callers. Fix this →


Stale alias in route loader

frontend/src/routes/mcp-servers/index.tsx:17

import { listMCPServers as listMCPServersV1 } from 'protogen/redpanda/api/dataplane/v1/mcp-MCPServerService_connectquery';

The V1 alias was introduced to distinguish from V1Alpha3. Since v1alpha3 is now gone, this should just be listMCPServers (used unaliased). Nit, but it's leftover noise from the refactor. Fix this →


Potential behavioral regression for serverless: connect client guide

frontend/src/components/pages/mcp-servers/details/remote-mcp-connection-tab.tsx

Before:

{(!isServerless() || isFeatureFlagEnabled('enableRemoteMcpConnectClientInConsoleServerless')) && (
  <RemoteMCPConnectClientGuide ... />
)}

After:

{!isServerless() && <RemoteMCPConnectClientGuide ... />}

If enableRemoteMcpConnectClientInConsoleServerless was set to true in the deployed configs (as the PR description implies — "already enabled"), then:

  • Before: (!isServerless() || true) → always rendered
  • After: !isServerless() → hidden for serverless users

This silently removes the connect client guide for serverless environments. If that's intentional (deciding serverless shouldn't see it), it's fine — but it's not mentioned in the PR description and contradicts the stated motivation of "flags already enabled = keep current behavior".


Minor: unused GetMCPServerResponse import

frontend/src/react-query/api/remote-mcp.tsx:14

type GetMCPServerResponse,

GetMCPServerResponse is imported but only used in the useGetMCPServerQuery signature via QueryOptions<GenMessage<GetMCPServerResponse>>. That's fine — just confirming this one is correctly typed unlike useListMCPServersQuery.


What looks good

  • The dual v1/v1alpha3 dispatch pattern removal in remote-mcp.tsx is clean and correct. All query keys, cache invalidations, and mutations now consistently target the v1 schema.
  • The console-app.tsx cleanup (removing embeddedAvailableRoutesObservable usage and its corresponding uiState breadcrumb sync) aligns with the prior commit that removed embeddedAvailableRoutesObservable.
  • remote-mcp-configuration-tab.tsx correctly adds optional chaining (tags?.) when removing the feature-flag wrapper: mcpServerData?.mcpServer?.tags?.[...] — the outer flag guard was previously providing implicit safety.
  • Test mocks are updated consistently across all affected test files.
  • The supported-features.ts addition of Feature.GetQuotas.endpoint to the return false list is orthogonal but reasonable to include here.

@malinskibeniamin malinskibeniamin requested review from a team February 20, 2026 20:35

export function isAdpEnabled() {
return config.isAdpEnabled;
return config.isAdpEnabled && !isServerless();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Serverless is completely out of the picture when it comes to ADP for now.

await queryClient.ensureQueryData(
createQueryOptions(listMCPServers, create(ListMCPServersRequestSchema, { pageSize: 50 }), {
transport: dataplaneTransport,
})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MCP is v1 so we should not use v1alpha3 anymore.

Copy link
Contributor

@sago2k8 sago2k8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please review Claude's comments, otherwise lgtm

@malinskibeniamin
Copy link
Contributor Author

Already applied claude review

@malinskibeniamin malinskibeniamin merged commit cd5942d into master Feb 23, 2026
19 checks passed
@malinskibeniamin malinskibeniamin deleted the feature-flag-cleanup branch February 23, 2026 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants