Skip to content

Commit ab11992

Browse files
alenkaczalenkacz
committed
fix: address code review feedback
- Fix broken service account caching by clearing cache when SA name changes - Fix form validation to only check SA field when feature flag enabled - Add missing MCPServer_Tool_ComponentType import for v1alpha3 - Export MCPServer enums from remote-mcp hooks for consistency - Add comment about resource lifecycle (matches AI agents pattern) - Import enums from unified hooks instead of directly from proto files
1 parent c2a99f6 commit ab11992

File tree

9 files changed

+264
-87
lines changed

9 files changed

+264
-87
lines changed

‎frontend/src/components/pages/agents/details/ai-agent-configuration-tab.tsx‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,9 @@ import {
4343
AIAgentUpdateSchema,
4444
UpdateAIAgentRequestSchema,
4545
} from 'protogen/redpanda/api/dataplane/v1alpha3/ai_agent_pb';
46-
import type { MCPServer } from 'protogen/redpanda/api/dataplane/v1alpha3/mcp_pb';
4746
import { useCallback, useMemo, useState } from 'react';
4847
import { useGetAIAgentQuery, useUpdateAIAgentMutation } from 'react-query/api/ai-agent';
49-
import { useListMCPServersQuery } from 'react-query/api/remote-mcp';
48+
import { type MCPServer, useListMCPServersQuery } from 'react-query/api/remote-mcp';
5049
import { useListSecretsQuery } from 'react-query/api/secret';
5150
import { useParams } from 'react-router-dom';
5251
import { toast } from 'sonner';

‎frontend/src/components/pages/mcp-servers/create/remote-mcp-create-page.tsx‎

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,7 @@ import { ExpandedYamlDialog } from 'components/ui/yaml/expanded-yaml-dialog';
2828
import { useYamlLabelSync } from 'components/ui/yaml/use-yaml-label-sync';
2929
import { config, isFeatureFlagEnabled } from 'config';
3030
import { ArrowLeft, FileText, Hammer, Loader2 } from 'lucide-react';
31-
import {
32-
CreateMCPServerRequestSchema as CreateMCPServerRequestSchemaV1,
33-
LintMCPConfigRequestSchema as LintMCPConfigRequestSchemaV1,
34-
MCPServer_ServiceAccountSchema,
35-
} from 'protogen/redpanda/api/dataplane/v1/mcp_pb';
36-
import {
37-
CreateMCPServerRequestSchema as CreateMCPServerRequestSchemaV1Alpha3,
38-
LintMCPConfigRequestSchema as LintMCPConfigRequestSchemaV1Alpha3,
39-
} from 'protogen/redpanda/api/dataplane/v1alpha3/mcp_pb';
31+
import { MCPServer_ServiceAccountSchema } from 'protogen/redpanda/api/dataplane/v1/mcp_pb';
4032
import React, { useEffect, useMemo, useRef, useState } from 'react';
4133
import { useFieldArray, useForm } from 'react-hook-form';
4234
import { useCreateMCPServerMutation, useLintMCPConfigMutation } from 'react-query/api/remote-mcp';
@@ -120,6 +112,13 @@ export const RemoteMCPCreatePage: React.FC = () => {
120112
}
121113
}, [displayName, form]);
122114

115+
// Clear cached service account when service account name changes
116+
useEffect(() => {
117+
if (serviceAccountInfo && serviceAccountName) {
118+
setServiceAccountInfo(null);
119+
}
120+
}, [serviceAccountName, serviceAccountInfo]);
121+
123122
const {
124123
fields: tagFields,
125124
append: appendTag,
@@ -159,7 +158,7 @@ export const RemoteMCPCreatePage: React.FC = () => {
159158

160159
const handleNext = async (isOnMetadataStep: boolean, goNext: () => void) => {
161160
if (isOnMetadataStep) {
162-
const fieldsToValidate = ['displayName', 'description', 'resourcesTier', 'tags'];
161+
const fieldsToValidate: Array<keyof FormValues> = ['displayName', 'description', 'resourcesTier', 'tags'];
163162
if (isFeatureFlagEnabled('enableMcpServiceAccount')) {
164163
fieldsToValidate.push('serviceAccountName');
165164
}
@@ -178,19 +177,16 @@ export const RemoteMCPCreatePage: React.FC = () => {
178177
return;
179178
}
180179

181-
const useMcpV1 = isFeatureFlagEnabled('enableMcpServiceAccount');
182180
const toolsMap: Record<string, { componentType: number; configYaml: string }> = {
183181
[tool.name.trim()]: {
184182
componentType: tool.componentType,
185183
configYaml: tool.config,
186184
},
187185
};
188186

189-
const response = await lintConfig(
190-
create(useMcpV1 ? LintMCPConfigRequestSchemaV1 : LintMCPConfigRequestSchemaV1Alpha3, {
191-
tools: toolsMap,
192-
})
193-
);
187+
const response = await lintConfig({
188+
tools: toolsMap,
189+
});
194190

195191
// Update lint hints for this tool
196192
setLintHints((prev) => ({
@@ -310,6 +306,9 @@ export const RemoteMCPCreatePage: React.FC = () => {
310306
let serviceAccountConfig: ReturnType<typeof create<typeof MCPServer_ServiceAccountSchema>> | undefined;
311307

312308
// Create service account if feature flag is enabled
309+
// NOTE: Service account and secret are created before MCP server creation.
310+
// If server creation fails, these resources will not be automatically cleaned up.
311+
// This matches the AI agents implementation pattern.
313312
if (useMcpServiceAccount) {
314313
const serviceAccountResult = await createServiceAccountIfNeeded(values.displayName);
315314
if (!serviceAccountResult) {
@@ -341,9 +340,9 @@ export const RemoteMCPCreatePage: React.FC = () => {
341340
};
342341

343342
await createServer(
344-
create(useMcpServiceAccount ? CreateMCPServerRequestSchemaV1 : CreateMCPServerRequestSchemaV1Alpha3, {
343+
{
345344
mcpServer: mcpServerPayload,
346-
}),
345+
},
347346
{
348347
onError: handleValidationError,
349348
onSuccess: (data) => {

‎frontend/src/components/pages/mcp-servers/create/schemas.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*/
1010

1111
import { RESOURCE_TIERS } from 'components/ui/connect/resource-tier-select';
12-
import { MCPServer_Tool_ComponentType } from 'protogen/redpanda/api/dataplane/v1alpha3/mcp_pb';
12+
import { MCPServer_Tool_ComponentType } from 'react-query/api/remote-mcp';
1313
import { parse } from 'yaml';
1414
import { z } from 'zod';
1515

‎frontend/src/components/pages/mcp-servers/create/use-metadata-validation.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,15 @@
88
* by the Apache License, Version 2.0
99
*/
1010

11+
import { isFeatureFlagEnabled } from 'config';
1112
import { useMemo } from 'react';
1213
import type { UseFormReturn } from 'react-hook-form';
1314

1415
import type { FormValues } from './schemas';
1516

1617
export function useMetadataValidation(form: UseFormReturn<FormValues>) {
1718
const formValues = form.watch();
19+
const isServiceAccountEnabled = isFeatureFlagEnabled('enableMcpServiceAccount');
1820

1921
const isMetadataComplete = useMemo(() => {
2022
// Check displayName (required)
@@ -33,13 +35,14 @@ export function useMetadataValidation(form: UseFormReturn<FormValues>) {
3335
form.formState.errors.description ||
3436
form.formState.errors.resourcesTier ||
3537
form.formState.errors.tags ||
36-
form.formState.errors.serviceAccountName
38+
(isServiceAccountEnabled && form.formState.errors.serviceAccountName)
3739
);
3840

3941
return !hasMetadataErrors;
4042
}, [
4143
formValues.displayName,
4244
formValues.resourcesTier,
45+
isServiceAccountEnabled,
4346
form.formState.errors.displayName,
4447
form.formState.errors.description,
4548
form.formState.errors.resourcesTier,

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

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,6 @@ import { isFeatureFlagEnabled } from 'config';
3838
import { Edit, FileText, Hammer, Plus, Save, Settings, ShieldCheck, Trash2 } from 'lucide-react';
3939
import type { LintHint } from 'protogen/redpanda/api/common/v1/linthint_pb';
4040
import { Scope } from 'protogen/redpanda/api/dataplane/v1/secret_pb';
41-
import {
42-
LintMCPConfigRequestSchema,
43-
UpdateMCPServerRequestSchema,
44-
} from 'protogen/redpanda/api/dataplane/v1alpha3/mcp_pb';
4541
import React, { useCallback, useEffect, useState } from 'react';
4642
import {
4743
type MCPServer_State,
@@ -63,7 +59,7 @@ import { type Template, templates } from '../templates/remote-mcp-templates';
6359
type LocalTool = {
6460
id: string;
6561
name: string;
66-
componentType: MCPServer_Tool_ComponentType;
62+
componentType: (typeof MCPServer_Tool_ComponentType)[keyof typeof MCPServer_Tool_ComponentType];
6763
config: string;
6864
selectedTemplate?: string;
6965
};
@@ -77,7 +73,7 @@ type LocalMCPServer = {
7773
tier: string;
7874
};
7975
tools: LocalTool[];
80-
state: MCPServer_State;
76+
state: (typeof MCPServer_State)[keyof typeof MCPServer_State];
8177
status: string;
8278
url: string;
8379
};
@@ -199,7 +195,7 @@ export const RemoteMCPConfigurationTab = () => {
199195
}
200196

201197
await updateMCPServer(
202-
create(UpdateMCPServerRequestSchema, {
198+
{
203199
id,
204200
mcpServer: {
205201
displayName: currentData.displayName,
@@ -216,7 +212,7 @@ export const RemoteMCPConfigurationTab = () => {
216212
updateMask: create(FieldMaskSchema, {
217213
paths: ['display_name', 'description', 'tools', 'tags', 'resources'],
218214
}),
219-
}),
215+
},
220216
{
221217
onError: (error) => {
222218
toast.error(formatToastErrorMessageGRPC({ error, action: 'update', entity: 'MCP server' }));
@@ -347,11 +343,9 @@ export const RemoteMCPConfigurationTab = () => {
347343
},
348344
};
349345

350-
const response = await lintConfig(
351-
create(LintMCPConfigRequestSchema, {
352-
tools: toolsMap,
353-
})
354-
);
346+
const response = await lintConfig({
347+
tools: toolsMap,
348+
});
355349

356350
// Update lint hints for this tool
357351
setLintHints((prev) => ({
@@ -383,11 +377,9 @@ export const RemoteMCPConfigurationTab = () => {
383377
},
384378
};
385379

386-
const response = await lintConfig(
387-
create(LintMCPConfigRequestSchema, {
388-
tools: toolsMap,
389-
})
390-
);
380+
const response = await lintConfig({
381+
tools: toolsMap,
382+
});
391383

392384
// Store hints for this tool
393385
if (response.lintHints && Object.keys(response.lintHints).length > 0) {
@@ -799,7 +791,10 @@ export const RemoteMCPConfigurationTab = () => {
799791
<Label className="font-medium text-sm">Component Type</Label>
800792
<Select
801793
onValueChange={(value) => {
802-
const componentType = Number.parseInt(value, 10) as MCPServer_Tool_ComponentType;
794+
const componentType = Number.parseInt(
795+
value,
796+
10
797+
) as (typeof MCPServer_Tool_ComponentType)[keyof typeof MCPServer_Tool_ComponentType];
803798
handleUpdateTool(selectedTool.id, { componentType });
804799
}}
805800
value={selectedTool.componentType.toString()}
@@ -818,7 +813,9 @@ export const RemoteMCPConfigurationTab = () => {
818813
.map((componentType) => (
819814
<SelectItem key={componentType} value={componentType.toString()}>
820815
<RedpandaConnectComponentTypeBadge
821-
componentType={componentType as MCPServer_Tool_ComponentType}
816+
componentType={
817+
componentType as (typeof MCPServer_Tool_ComponentType)[keyof typeof MCPServer_Tool_ComponentType]
818+
}
822819
/>
823820
</SelectItem>
824821
))}

‎frontend/src/components/pages/mcp-servers/list/remote-mcp-list-page.tsx‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,10 @@ import { Text } from 'components/redpanda-ui/components/typography';
4747
import { DeleteResourceAlertDialog } from 'components/ui/delete-resource-alert-dialog';
4848
import { AlertCircle, Check, Copy, Loader2, MoreHorizontal, Pause, Play, Plus, X } from 'lucide-react';
4949
import { runInAction } from 'mobx';
50-
import type { MCPServer as APIMCPServer } from 'protogen/redpanda/api/dataplane/v1alpha3/mcp_pb';
51-
import { MCPServer_State } from 'protogen/redpanda/api/dataplane/v1alpha3/mcp_pb';
5250
import React, { useEffect } from 'react';
5351
import {
52+
type MCPServer as APIMCPServer,
53+
MCPServer_State,
5454
useDeleteMCPServerMutation,
5555
useListMCPServersQuery,
5656
useStartMCPServerMutation,
@@ -72,13 +72,21 @@ export type MCPServer = {
7272
id: string;
7373
name: string;
7474
url: string;
75-
state: MCPServer_State;
75+
state: (typeof MCPServer_State)[keyof typeof MCPServer_State];
7676
tools: string[];
7777
lastConnected?: string;
7878
};
7979

80-
const StatusIcon = ({ state }: { state: MCPServer_State }) => {
81-
const statusProps = {
80+
const StatusIcon = ({ state }: { state: (typeof MCPServer_State)[keyof typeof MCPServer_State] }) => {
81+
const statusPropsMap: Record<
82+
number,
83+
{
84+
text: string;
85+
icon: React.ComponentType<{ className?: string }>;
86+
iconColor: string;
87+
animate?: boolean;
88+
}
89+
> = {
8290
[MCPServer_State.RUNNING]: {
8391
text: 'Running',
8492
icon: Check,
@@ -111,7 +119,9 @@ const StatusIcon = ({ state }: { state: MCPServer_State }) => {
111119
icon: AlertCircle,
112120
iconColor: 'text-gray-500',
113121
},
114-
}[state] || {
122+
};
123+
124+
const statusProps = statusPropsMap[state] || {
115125
text: 'Unknown',
116126
icon: AlertCircle,
117127
iconColor: 'text-red-600',

‎frontend/src/components/ui/mcp/mcp-server-card.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { MCPServer } from 'protogen/redpanda/api/dataplane/v1alpha3/mcp_pb';
1+
import type { MCPServer } from 'react-query/api/remote-mcp';
22
import type { HTMLAttributes } from 'react';
33
import { useMemo } from 'react';
44

‎frontend/src/components/ui/mcp/mcp-server-state-badge.tsx‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,10 @@
1111

1212
import { Badge, type BadgeVariant } from 'components/redpanda-ui/components/badge';
1313
import { AlertCircle, Check, Clock, Loader2, StopCircle } from 'lucide-react';
14-
import { MCPServer_State } from 'protogen/redpanda/api/dataplane/v1alpha3/mcp_pb';
15-
import { useGetMCPServerQuery } from 'react-query/api/remote-mcp';
14+
import { MCPServer_State, useGetMCPServerQuery } from 'react-query/api/remote-mcp';
1615
import { useParams } from 'react-router-dom';
1716

18-
const getMCPServerStatus = (state: MCPServer_State): { icon: React.ReactNode; text: string; variant: BadgeVariant } => {
17+
const getMCPServerStatus = (state: typeof MCPServer_State[keyof typeof MCPServer_State]): { icon: React.ReactNode; text: string; variant: BadgeVariant } => {
1918
switch (state) {
2019
case MCPServer_State.RUNNING:
2120
return {

0 commit comments

Comments
 (0)