Skip to content

Commit c8fe6d8

Browse files
committed
Refactor MCP Proxy components for improved state management and endpoint handling
1 parent f3ca4f3 commit c8fe6d8

7 files changed

Lines changed: 89 additions & 57 deletions

File tree

console/workspaces/libs/api-client/src/apis/scopes.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
* under the License.
1717
*/
1818

19-
import { httpDELETE, httpGET, httpPOST, httpPUT, SERVICE_BASE } from "../utils";
19+
import { encodeRequired, httpDELETE, httpGET, httpPOST, httpPUT, SERVICE_BASE } from "../utils";
2020
import type {
2121
ScopeListResponse,
2222
ScopeResponse,
@@ -69,10 +69,9 @@ export async function updateScope(
6969
getToken?: () => Promise<string>,
7070
): Promise<ScopeResponse> {
7171
const { orgName = "default", scopeName } = params;
72+
const scope = encodeRequired(scopeName, "scopeName");
7273
const token = getToken ? await getToken() : undefined;
73-
const res = await httpPUT(
74-
`${orgBase(orgName)}/${encodeURIComponent(scopeName ?? "")}`, body, { token },
75-
);
74+
const res = await httpPUT(`${orgBase(orgName)}/${scope}`, body, { token });
7675
if (!res.ok) throw await res.json();
7776
return res.json();
7877
}
@@ -86,9 +85,8 @@ export async function deleteScope(
8685
getToken?: () => Promise<string>,
8786
): Promise<void> {
8887
const { orgName = "default", scopeName } = params;
88+
const scope = encodeRequired(scopeName, "scopeName");
8989
const token = getToken ? await getToken() : undefined;
90-
const res = await httpDELETE(
91-
`${orgBase(orgName)}/${encodeURIComponent(scopeName ?? "")}`, { token },
92-
);
90+
const res = await httpDELETE(`${orgBase(orgName)}/${scope}`, { token });
9391
if (!res.ok && res.status !== 204) throw await res.json();
9492
}

console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/RoleEditPage.tsx

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,13 @@ export const RoleEditPage: React.FC = () => {
133133

134134
useEffect(() => {
135135
if (!hasEditedScopes.current && catalogScopes.length > 0) {
136-
const nameSet = new Set(initialScopeNames);
137-
setSelectedScopes(catalogScopes.filter((s) => nameSet.has(s.name)));
136+
const catalogByName = new Map(catalogScopes.map((s) => [s.name, s]));
137+
// A scope assigned to this role may no longer be in the catalog (deleted
138+
// or renamed) — keep it as a placeholder so it isn't silently dropped
139+
// (and dirtied) on load, and stays in the payload if the role is saved.
140+
setSelectedScopes(
141+
initialScopeNames.map((name) => catalogByName.get(name) ?? { id: name, name }),
142+
);
138143
}
139144
}, [initialScopeNames, catalogScopes]);
140145

console/workspaces/pages/mcp-proxies/src/subComponents/EditMCPProxyDrawer.tsx

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
* under the License.
1616
*/
1717

18-
import React, { useCallback, useEffect, useMemo, useState } from "react";
18+
import React, { useCallback, useEffect, useMemo, useRef, useState } from "react";
1919
import {
2020
Alert,
2121
Box,
@@ -92,8 +92,16 @@ export function EditMCPProxyDrawer({
9292
const [addOpen, setAddOpen] = useState(false);
9393
const [editingId, setEditingId] = useState<string | null>(null);
9494

95+
const wasOpenRef = useRef(false);
96+
9597
useEffect(() => {
96-
if (!open) return;
98+
const justOpened = open && !wasOpenRef.current;
99+
wasOpenRef.current = open;
100+
// Only (re)seed on the closed→open transition — proxy can change while the
101+
// drawer is already open (e.g. a background refetch), and reseeding then
102+
// would clobber whatever the user is mid-editing.
103+
if (!justOpened) return;
104+
97105
setFormData({
98106
name: proxy.name,
99107
version: proxy.version,

console/workspaces/pages/mcp-proxies/src/subComponents/EndpointFormFields.tsx

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,11 +145,13 @@ export function EndpointFormFields({
145145
setFetchedInfo(null);
146146
}, []);
147147

148-
const performFetch = useCallback(async () => {
149-
// Guards against a debounced call landing while a previous fetch for this form
150-
// is still in flight (e.g. a slow request outlasting the debounce window).
151-
if (fetchServerInfo.isPending) return;
148+
// Identifies the latest performFetch call whose network request should be allowed
149+
// to update state. A slow request can still be in flight when the debounced URL/auth
150+
// change fires the next one; without this, whichever resolves last wins, which isn't
151+
// necessarily the latest input.
152+
const fetchGenerationRef = useRef(0);
152153

154+
const performFetch = useCallback(async () => {
153155
const urlValidationError = validateEndpointUrl(trimmedUrl, {
154156
requiredMessage: "Enter a valid MCP Proxy endpoint URL.",
155157
invalidMessage: "Enter a valid MCP Proxy endpoint URL.",
@@ -186,13 +188,18 @@ export function EndpointFormFields({
186188
? { url: trimmedUrl, auth: { type: "api-key" as const, header, value } }
187189
: { url: trimmedUrl };
188190

191+
const generation = ++fetchGenerationRef.current;
189192
try {
190193
const result = await fetchServerInfo.mutateAsync({
191194
params: { orgName: orgId },
192195
body,
193196
});
197+
// A newer fetch was started while this one was in flight — its result (or
198+
// the newer one still pending) takes precedence, so drop this stale one.
199+
if (generation !== fetchGenerationRef.current) return;
194200
setFetchedInfo(result);
195201
} catch (err: unknown) {
202+
if (generation !== fetchGenerationRef.current) return;
196203
setFetchedInfo(null);
197204
if (
198205
typeof err === "object" &&

console/workspaces/pages/mcp-proxies/src/subComponents/EndpointsEditorSection.tsx

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,25 @@
1515
* under the License.
1616
*/
1717

18-
import { useCallback, useMemo, useRef } from "react";
18+
import { useCallback, useMemo } from "react";
1919
import { Box, Button, Stack, Tooltip, Typography } from "@wso2/oxygen-ui";
2020
import { Plus } from "@wso2/oxygen-ui-icons-react";
2121
import type { Environment } from "@agent-management-platform/types";
2222
import { EndpointFormFields, type EndpointDraft } from "./EndpointFormFields";
2323
import { EndpointRow } from "./EndpointRow";
2424

25+
// Backend endpoint ids are handles derived from a user-chosen display name
26+
// (lowercased, alphanumeric + hyphen — see GenerateHandle server-side), so a
27+
// simple counter like "1" can collide with a real endpoint's id. A UUID can't
28+
// collide in practice, keeping new drafts from being mistaken for — and
29+
// merged with — an existing endpoint's saved config on submit.
30+
function createDraftEndpointId(): string {
31+
if (typeof crypto !== "undefined" && typeof crypto.randomUUID === "function") {
32+
return crypto.randomUUID();
33+
}
34+
return `draft-${Date.now()}-${Math.random().toString(36).slice(2)}`;
35+
}
36+
2537
export interface EndpointsEditorSectionProps {
2638
orgId: string;
2739
environments: Environment[];
@@ -54,8 +66,6 @@ export function EndpointsEditorSection({
5466
emptyStateText,
5567
onEndpointAdded,
5668
}: EndpointsEditorSectionProps) {
57-
const endpointIdRef = useRef(1);
58-
5969
const environmentLabels = useMemo(() => {
6070
const labels = new Map<string, string>();
6171
environments.forEach((env) => {
@@ -93,9 +103,7 @@ export function EndpointsEditorSection({
93103

94104
const handleAddEndpoint = useCallback(
95105
(draft: Omit<EndpointDraft, "id">) => {
96-
const id = String(endpointIdRef.current);
97-
endpointIdRef.current += 1;
98-
onEndpointsChange([...endpoints, { ...draft, id }]);
106+
onEndpointsChange([...endpoints, { ...draft, id: createDraftEndpointId() }]);
99107
onEndpointAdded?.(draft);
100108
onAddOpenChange(false);
101109
},

console/workspaces/pages/mcp-proxies/src/subComponents/MCPProxySecurityTab.tsx

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -112,27 +112,37 @@ export function MCPProxySecurityTab({
112112
} | null>(null);
113113
const [fieldErrors, setFieldErrors] = useState<{ keyValue?: string }>({});
114114

115+
// Tracks what was last confirmed persisted (seeded from config, refreshed on
116+
// save) rather than re-deriving "saved" straight from the config prop on
117+
// every render — config only reflects a save once its background refetch
118+
// lands, which would otherwise leave authIsDirty true for a beat after a
119+
// successful save.
120+
const lastSavedAuthRef = useRef<{
121+
type: AuthenticationType;
122+
key: string;
123+
in: APIKeyLocation;
124+
}>({ type: "apiKey", key: "", in: "header" });
125+
115126
const authIsDirty = useMemo(() => {
116127
if (!config) return false;
117-
const savedType = resolveAuthenticationType(config);
118-
const savedKey = config.security?.apiKey?.key ?? "";
119-
const savedIn = (config.security?.apiKey?.in as APIKeyLocation) ?? "header";
120-
if (authenticationType !== savedType) return true;
121-
if (keyValue.trim() !== savedKey) return true;
122-
if (keyIn !== savedIn) return true;
128+
const saved = lastSavedAuthRef.current;
129+
if (authenticationType !== saved.type) return true;
130+
if (keyValue.trim() !== saved.key) return true;
131+
if (keyIn !== saved.in) return true;
123132
return false;
124133
}, [config, authenticationType, keyValue, keyIn]);
125134

126135
useEffect(() => {
127136
if (!config || !selectedEndpointId) return;
128137
const nextType = resolveAuthenticationType(config);
138+
const nextKey =
139+
config.security?.apiKey?.key ?? (nextType === "apiKey" ? "X-API-Key" : "");
140+
const nextIn = (config.security?.apiKey?.in as APIKeyLocation) ?? "header";
129141
setAuthenticationType(nextType);
130-
setKeyValue(
131-
config.security?.apiKey?.key ??
132-
(nextType === "apiKey" ? "X-API-Key" : ""),
133-
);
134-
setKeyIn((config.security?.apiKey?.in as APIKeyLocation) ?? "header");
142+
setKeyValue(nextKey);
143+
setKeyIn(nextIn);
135144
setFieldErrors({});
145+
lastSavedAuthRef.current = { type: nextType, key: nextKey, in: nextIn };
136146
}, [config, selectedEndpointId]);
137147

138148
// --- Agent Identity: per-tool scope-binding (RBAC) state ---
@@ -363,6 +373,12 @@ export function MCPProxySecurityTab({
363373
toolScopeBindings: nextBindings,
364374
});
365375
lastSavedToolScopeRowsRef.current = savedToolScopeRows;
376+
setToolScopeRows(savedToolScopeRows);
377+
lastSavedAuthRef.current = {
378+
type: authenticationType,
379+
key: authenticationType === "apiKey" ? keyValue.trim() : "",
380+
in: keyIn,
381+
};
366382
setStatus({
367383
message: "Updated security settings.",
368384
severity: "success",

console/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsx

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -69,27 +69,17 @@ import { MCPProxySecurityTab } from "./MCPProxySecurityTab";
6969
import { EditMCPProxyDrawer } from "./EditMCPProxyDrawer";
7070
import { useCopyWithFeedback } from "./useCopyWithFeedback";
7171

72-
const TABS = [
73-
"Overview",
74-
"Capabilities",
75-
"Connection",
76-
"Manage Tools",
77-
"Security",
78-
"Rewrite",
79-
"Policies",
80-
] as const;
81-
82-
// URL-safe stand-ins for each tab, index-aligned with TABS, so the selected
83-
// tab (and environment, below) are shareable/deep-linkable and survive a
84-
// page reload instead of resetting to Overview/first-environment.
85-
const TAB_SLUGS = [
86-
"overview",
87-
"capabilities",
88-
"connection",
89-
"manage-tools",
90-
"security",
91-
"rewrite",
92-
"policies",
72+
// slug is a URL-safe stand-in for each tab, so the selected tab (and
73+
// environment, below) are shareable/deep-linkable and survive a page reload
74+
// instead of resetting to Overview/first-environment.
75+
const TAB_DEFS = [
76+
{ label: "Overview", slug: "overview" },
77+
{ label: "Capabilities", slug: "capabilities" },
78+
{ label: "Connection", slug: "connection" },
79+
{ label: "Manage Tools", slug: "manage-tools" },
80+
{ label: "Security", slug: "security" },
81+
{ label: "Rewrite", slug: "rewrite" },
82+
{ label: "Policies", slug: "policies" },
9383
] as const;
9484

9585
export function ViewMCPProxy() {
@@ -99,7 +89,7 @@ export function ViewMCPProxy() {
9989

10090
const tabSlug = searchParams.get("tab");
10191
const tabIndex = tabSlug
102-
? Math.max(0, TAB_SLUGS.indexOf(tabSlug as (typeof TAB_SLUGS)[number]))
92+
? Math.max(0, TAB_DEFS.findIndex((tab) => tab.slug === tabSlug))
10393
: 0;
10494
const selectedEndpointId = searchParams.get("endpoint") ?? "";
10595

@@ -122,7 +112,7 @@ export function ViewMCPProxy() {
122112
setSearchParams(
123113
(prev) => {
124114
const next = new URLSearchParams(prev);
125-
next.set("tab", TAB_SLUGS[value]);
115+
next.set("tab", TAB_DEFS[value].slug);
126116
return next;
127117
},
128118
{ replace: true },
@@ -373,8 +363,8 @@ export function ViewMCPProxy() {
373363
sx={{ pr: 2 }}
374364
>
375365
<Tabs value={tabIndex} onChange={handleTabChange}>
376-
{TABS.map((tab) => (
377-
<Tab key={tab} label={tab} />
366+
{TAB_DEFS.map((tab) => (
367+
<Tab key={tab.slug} label={tab.label} />
378368
))}
379369
</Tabs>
380370
<Typography

0 commit comments

Comments
 (0)