Skip to content

Implement MCP Proxy support#961

Open
Oshanath wants to merge 1 commit into
wso2:mainfrom
Oshanath:feature-mcp-proxy
Open

Implement MCP Proxy support#961
Oshanath wants to merge 1 commit into
wso2:mainfrom
Oshanath:feature-mcp-proxy

Conversation

@Oshanath
Copy link
Copy Markdown

@Oshanath Oshanath commented May 25, 2026

Summary by CodeRabbit

  • New Features
    • MCP proxy management with create, list, view, update, and delete operations
    • MCP server endpoint discovery and information retrieval
    • Agent MCP server bindings and configuration management
    • Policy catalog integration with parameter-based configuration editing
    • Automatic MCP proxy deployment to gateways
    • Runtime environment variable management for MCP servers
    • Policy validation and schema-based parameter editing support

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 25, 2026

Warning

Review limit reached

@Oshanath, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 16 minutes and 19 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3a9322d2-d713-4991-b07a-c97b330e18b2

📥 Commits

Reviewing files that changed from the base of the PR and between 7899ae6 and ca1f223.

⛔ Files ignored due to path filters (1)
  • console/common/config/rush/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (85)
  • agent-manager-service/api/agent_mcp_proxy_routes.go
  • agent-manager-service/api/app.go
  • agent-manager-service/api/gateway_internal_routes.go
  • agent-manager-service/api/mcp_proxy_routes.go
  • agent-manager-service/controllers/agent_mcp_proxy_controller.go
  • agent-manager-service/controllers/gateway_internal_controller.go
  • agent-manager-service/controllers/mcp_proxy_controller.go
  • agent-manager-service/db_migrations/023_create_mcp_proxies.go
  • agent-manager-service/db_migrations/migration_list.go
  • agent-manager-service/models/agent_mcp_proxy_mapping.go
  • agent-manager-service/models/artifact.go
  • agent-manager-service/models/gateway_compat.go
  • agent-manager-service/models/mcp_proxy.go
  • agent-manager-service/repositories/agent_mcp_proxy_mapping_repository.go
  • agent-manager-service/repositories/artifact_repository.go
  • agent-manager-service/repositories/mcp_proxy_repository.go
  • agent-manager-service/services/agent_manager.go
  • agent-manager-service/services/agent_mcp_proxy_service.go
  • agent-manager-service/services/deployment_ack_handler.go
  • agent-manager-service/services/gateway_events_service.go
  • agent-manager-service/services/gateway_internal_service.go
  • agent-manager-service/services/mcp_proxy_deployment.go
  • agent-manager-service/services/mcp_proxy_service.go
  • agent-manager-service/services/mcp_proxy_service_test.go
  • agent-manager-service/services/platform_gateway_service.go
  • agent-manager-service/utils/api.go
  • agent-manager-service/utils/constants.go
  • agent-manager-service/utils/errors.go
  • agent-manager-service/wiring/params.go
  • agent-manager-service/wiring/wire.go
  • agent-manager-service/wiring/wire_gen.go
  • console/apps/web-ui/vite.config.ts
  • console/rush.json
  • console/workspaces/core-ui/package.json
  • console/workspaces/core-ui/src/Layouts/OxygenLayout/navigationItems.tsx
  • console/workspaces/core-ui/src/Route/Route.tsx
  • console/workspaces/core-ui/src/pages/index.tsx
  • console/workspaces/core-ui/vite.config.ts
  • console/workspaces/libs/api-client/src/apis/agent-mcp-proxies.ts
  • console/workspaces/libs/api-client/src/apis/index.ts
  • console/workspaces/libs/api-client/src/apis/mcp-proxies.ts
  • console/workspaces/libs/api-client/src/hooks/agent-mcp-proxies.ts
  • console/workspaces/libs/api-client/src/hooks/index.ts
  • console/workspaces/libs/api-client/src/hooks/mcp-policies.ts
  • console/workspaces/libs/api-client/src/hooks/mcp-proxies.ts
  • console/workspaces/libs/shared-component/package.json
  • console/workspaces/libs/shared-component/src/index.ts
  • console/workspaces/libs/shared-component/src/utils/policyParameterEditor/index.ts
  • console/workspaces/libs/shared-component/src/utils/policyParameterEditor/schemaUtils.ts
  • console/workspaces/libs/shared-component/src/utils/policyParameterEditor/types.ts
  • console/workspaces/libs/shared-component/src/utils/policyParameterEditor/yamlParser.ts
  • console/workspaces/libs/types/src/api/agent-mcp-proxies.ts
  • console/workspaces/libs/types/src/api/index.ts
  • console/workspaces/libs/types/src/api/mcp-proxies.ts
  • console/workspaces/libs/types/src/routes/generated-route.map.ts
  • console/workspaces/libs/types/src/routes/routes.map.ts
  • console/workspaces/pages/configure-agent/package.json
  • console/workspaces/pages/configure-agent/src/AddMCPServer.Component.tsx
  • console/workspaces/pages/configure-agent/src/Configure.Component.tsx
  • console/workspaces/pages/configure-agent/src/Configure/subComponents/AgentMCPServersSection.tsx
  • console/workspaces/pages/configure-agent/src/ViewMCPServer.Component.tsx
  • console/workspaces/pages/configure-agent/src/index.ts
  • console/workspaces/pages/llm-providers/src/PolicyParameterEditor/FieldRenderers.tsx
  • console/workspaces/pages/llm-providers/src/PolicyParameterEditor/PolicyParameterEditor.tsx
  • console/workspaces/pages/llm-providers/src/PolicyParameterEditor/SchemaTree.tsx
  • console/workspaces/pages/llm-providers/src/PolicyParameterEditor/yamlParser.ts
  • console/workspaces/pages/llm-providers/src/components/GuardrailSelectorDrawer/GuardrailSelectorDrawer.tsx
  • console/workspaces/pages/llm-providers/src/subComponents/GuardrailsSection.tsx
  • console/workspaces/pages/llm-providers/src/subComponents/LLMProviderGuardrailsTab.tsx
  • console/workspaces/pages/mcp-proxies/eslint.config.js
  • console/workspaces/pages/mcp-proxies/package.json
  • console/workspaces/pages/mcp-proxies/src/AddMCPProxy.Organization.tsx
  • console/workspaces/pages/mcp-proxies/src/MCPProxies.Organization.tsx
  • console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/FieldRenderers.tsx
  • console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/PolicyParameterEditor.tsx
  • console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/SchemaTree.tsx
  • console/workspaces/pages/mcp-proxies/src/components/MCPCapabilitiesView.tsx
  • console/workspaces/pages/mcp-proxies/src/components/MCPLogo.tsx
  • console/workspaces/pages/mcp-proxies/src/index.ts
  • console/workspaces/pages/mcp-proxies/src/subComponents/AddMCPProxyForm.tsx
  • console/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyTable.tsx
  • console/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsx
  • console/workspaces/pages/mcp-proxies/tsconfig.json
  • console/workspaces/pages/mcp-proxies/tsconfig.lib.json
  • console/workspaces/pages/mcp-proxies/vite.config.ts
📝 Walkthrough

Walkthrough

Adds MCP Proxy feature end-to-end: DB migration/models, repositories/services (CRUD, discovery, deployment, env sync), HTTP routes/controllers, gateway internal APIs, wiring, and full console UI with types, API clients/hooks, routes, and policy parameter editors.

Changes

MCP Proxies and Agent Bindings

Layer / File(s) Summary
Backend schema and models
agent-manager-service/db_migrations/*, agent-manager-service/models/*
Creates mcp_proxies and agent_mcp_proxy_mappings tables; adds MCP proxy, mapping, artifact kind, compatibility DTOs, and server info/policy DTOs.
Persistence and services
agent-manager-service/repositories/*, agent-manager-service/services/mcp_*, agent-manager-service/services/gateway_*, agent-manager-service/services/deployment_ack_handler.go
Implements repositories and services for MCP proxies and agent mappings, deployment/redeploy/broadcast flows, env var sync, server discovery, and internal gateway fetch/secret resolution.
HTTP API and wiring
agent-manager-service/api/*, agent-manager-service/controllers/*, agent-manager-service/wiring/*, agent-manager-service/utils/*
Adds public/org/agent routes and controllers, internal gateway route, DI wiring, constants/errors, and YAML ZIP helper.
Agent manager integration
agent-manager-service/services/agent_manager.go
Integrates MCP mapping cleanup and deploy-time MCP_SERVER_URLS rebuilding.
Console types and API clients/hooks
console/workspaces/libs/types/*, console/workspaces/libs/api-client/*
Adds MCP proxy and agent-mapping types, clients, and React Query hooks.
Console UI and routing
console/workspaces/core-ui/*, console/.../pages/mcp-proxies/*, console/.../pages/configure-agent/*
Adds org-level MCP proxies listing/add/view, agent MCP server binding add/view/edit, navigation/routes, and policy parameter editor components/utilities.
Build/config updates
console/apps/web-ui/vite.config.ts, console/workspaces/*/vite.config.ts, console/rush.json, console/.../package.json
Wires workspace, aliases, package manifests, ESLint/TS configs for new pages and shared utils.

Sequence Diagram(s)

sequenceDiagram
  participant UI as Console UI
  participant API as REST Controllers
  participant SVC as MCPProxy/AgentMCP Services
  participant DB as Repos/DB
  participant GW as Gateway Internal

  UI->>API: Create MCP Proxy / Bind Agent
  API->>SVC: Validate + execute
  SVC->>DB: Persist proxy/mapping
  SVC-->>GW: Deploy/redeploy/broadcast
  GW-->>UI: Fetch YAML (internal flow)
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

  • wso2/agent-manager#949: Uses the same GatewayEventsService path to deliver events, directly relevant to new MCP proxy broadcast methods.

Suggested reviewers

  • menakaj
  • hanzjk
  • rasika2012

Poem

I built a bridge of YAML and Go,
Proxies hop where the mappings flow.
Gateways whisper zipped secrets tight,
Console tabs glow in Oxygen light.
Policies dance, versions align—
A rabbit ships MCP, right on time. 🐇✨

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 17

🧹 Nitpick comments (1)
console/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyTable.tsx (1)

300-320: ⚡ Quick win

Consolidate duplicated display helpers to prevent UI drift.

twoLetters and formatRelativeTime are duplicated across components and already diverge (for example, Line [304] lowercases the second initial, and Line [313] differs in casing from the other page). Please move these helpers to a shared util and reuse them.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@console/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyTable.tsx`
around lines 300 - 320, The twoLetters and formatRelativeTime helper
implementations are duplicated and have diverged; extract both functions
(twoLetters and formatRelativeTime) into a shared utility module (e.g., a new
util file exported as named functions) and update MCPProxyTable.tsx to import
and use those shared helpers instead of the local definitions; ensure you
preserve the intended behavior (keep the current casing rules used in this file
or reconcile to the canonical behavior) and remove the duplicate local functions
so other components can import the same implementations to avoid UI drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@agent-manager-service/controllers/gateway_internal_controller.go`:
- Around line 349-362: The handler currently allows unauthenticated manifest
writes when apiKey is empty; update the logic around apiKey /
c.gatewayService.VerifyToken to require an API key: if r.Header.Get("api-key")
is empty, immediately log and respond with http.StatusUnauthorized, and only
call c.gatewayService.VerifyToken(apiKey) when a key is present; keep the
existing checks that compare gateway.UUID.String() to gatewayID and return
http.StatusForbidden on mismatch, and preserve the existing warn logs (refer to
symbols apiKey, c.gatewayService.VerifyToken, gateway.UUID.String(), and
gatewayID).
- Around line 370-373: When SaveGatewayPolicyManifest returns an error, check
whether the error equals utils.ErrGatewayNotFound and, in that case, log and
return http.StatusNotFound (404) instead of http.StatusInternalServerError;
otherwise keep the existing 500 behavior. Update the error handling around
gatewayService.SaveGatewayPolicyManifest(gatewayID, manifest) to compare the
returned error against utils.ErrGatewayNotFound (using errors.Is or direct
equality as appropriate), call log.Error with context, and call http.Error(w,
"Gateway not found", http.StatusNotFound) for that specific case.

In `@agent-manager-service/controllers/mcp_proxy_controller.go`:
- Line 68: Replace the hardcoded "system" actor passed to mcpProxyService.Create
with the authenticated actor from the request context: obtain the actor (e.g.,
via the controller's context helper or a context value like an auth/actor key)
and pass that actor string as the third parameter to mcpProxyService.Create
instead of "system"; if no actor is present, fall back to a safe default, but
prefer propagating the real request actor so audit/ownership is preserved
(update the call to resp, err := c.mcpProxyService.Create(ctx, orgName, actor,
&req)).
- Around line 243-257: FetchServerInfo currently relies only on
validateMCPServerURL and calls http.NewRequestWithContext then s.client.Do,
which allows SSRF to internal addresses; update mcp_proxy_service.go to (1)
harden validateMCPServerURL (or add a new validateMCPServerTarget) to reject
hostnames that equal "localhost" and literal loopback IPv4/IPv6, and (2) perform
DNS resolution (net.LookupIP or net.DefaultResolver.LookupIPAddr/netip) of the
target hostname and reject any resolved IPs that are loopback, private, or
link-local (use IP.IsLoopback/IsPrivate or netip.Addr methods), returning
utils.ErrInvalidURL/ErrURLUnreachable as appropriate; and (3) replace the plain
http.Client usage around http.NewRequestWithContext / s.client.Do with a client
that uses a custom Transport.DialContext which checks the resolved IP against
the same allowlist/denylist before dialing (or uses a DialContext that rejects
non-public IPs) to prevent race conditions after DNS resolution. Ensure the
checks are applied in the code paths used by FetchServerInfo so internal
addresses are rejected and proper error responses are returned.

In `@agent-manager-service/models/agent_mcp_proxy_mapping.go`:
- Around line 127-129: AgentMCPMappingArtifactHandle currently concatenates and
then slugifies ("agent-mcp-"+projectName+"-"+agentID+"-"+name), which is lossy
and can produce collisions; change it to produce a collision-resistant handle by
keeping per-component boundaries and appending a short deterministic digest of
the raw inputs: keep using slugifyMCPMappingPart for a readable prefix (e.g.,
slugified components separated clearly) and then append a stable short
fingerprint (e.g., first N chars of sha256/hex or base64url of
projectName+":"+agentID+":"+name) to guarantee uniqueness; update the
implementation that references slugifyMCPMappingPart (and the sibling functions
in the same file noted around lines 131-166) to follow the same pattern so all
slug-based keys include a digest suffix.

In `@agent-manager-service/repositories/mcp_proxy_repository.go`:
- Around line 191-193: Count currently uses artifactRepo.CountByKindAndOrg which
includes mapping artifacts (persisted as models.KindMCPProxy in
agent_mcp_proxy_mapping_repository.go) and thus inflates totals; change
MCPProxyRepo.Count to count actual mcp_proxies rows instead of artifacts by
either calling a dedicated mcp proxy table count method (e.g., add/use a
CountMCPProxiesByOrg on the DB/repo) or update the artifactRepo call to exclude
mapping artifacts by a discriminating field (metadata/origin) so only true
mcp_proxies are counted; reference MCPProxyRepo.Count,
artifactRepo.CountByKindAndOrg and the mapping behavior in
agent_mcp_proxy_mapping_repository.go when implementing the fix.

In `@agent-manager-service/services/mcp_proxy_service.go`:
- Around line 854-865: validateMCPServerURL currently only checks scheme/host
and thus allows SSRF to internal addresses; update validateMCPServerURL (and
call it from FetchServerInfo before any outbound request) to reject hosts that
resolve to local/private addresses by: parse the host (handle host:port), if
host is an IP literal (net.ParseIP) or resolves via net.LookupIP, ensure none of
the resolved IPs are in loopback (127.0.0.0/8, ::1), link-local (169.254.0.0/16,
fe80::/10), RFC1918/private ranges (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16,
100.64.0.0/10), unique-local IPv6 (fc00::/7) or known metadata IPs
(169.254.169.254), and reject the URL with an error if any match; keep scheme
checks for http/https and return clear errors referencing validateMCPServerURL
and FetchServerInfo.

In `@console/workspaces/libs/types/src/api/agent-mcp-proxies.ts`:
- Line 57: The path param types allow proxyId to be undefined which is
incorrect; update UpdateAgentMCPProxyPathParams (and the corresponding
RemoveAgentMCPProxyPathParams) so that proxyId is required (remove the optional
`?`) and typed as string (e.g., { proxyId: string }), ensuring both types extend
AgentPathParams and force callers to provide proxyId for the update/remove
endpoints.

In `@console/workspaces/libs/types/src/api/mcp-proxies.ts`:
- Around line 100-102: The three path param types DeleteMCPProxyPathParams,
GetMCPProxyPathParams, and UpdateMCPProxyPathParams currently mark proxyId as
optional; change each to require proxyId (remove the optional marker) so the
types are { proxyId: string } combined with OrgPathParams, ensuring callers must
provide proxyId at compile time.

In `@console/workspaces/pages/configure-agent/src/AddMCPServer.Component.tsx`:
- Around line 99-100: Add an unmount cleanup that clears the pending debounce
timeout stored in searchTimerRef to prevent setDebouncedSearch from running
after the component is unmounted: inside a useEffect with no dependencies,
return a cleanup function that checks searchTimerRef.current, calls clearTimeout
if present, and sets searchTimerRef.current to null so pending timers are
canceled and cannot trigger setDebouncedSearch post-unmount.

In `@console/workspaces/pages/mcp-proxies/src/AddMCPProxy.Organization.tsx`:
- Line 30: Update the awkward backLabel text in AddMCPProxy.Organization.tsx:
replace the current backLabel="Back to Create List" with a clearer destination
such as backLabel="Back to List" (or "Back to Create" if the intent is returning
to the create form). Locate the prop named backLabel in the component render and
change the string accordingly so the label accurately reflects the navigation
target.

In
`@console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/schemaUtils.ts`:
- Around line 143-151: The code descends into current[key] without replacing
primitives, so before setting current = current[key] ensure intermediate
primitives are replaced: in the block around current[key], treat any non-object
(typeof current[key] !== "object" || current[key] === null) as missing and
assign isNextKeyArrayIndex ? [] : {} (in addition to the existing undefined/null
check), then preserve the array copy branch (Array.isArray) and object copy
branch; this prevents runtime throws when setValueByPath (or related functions)
tries to descend into a primitive.

In
`@console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/yamlParser.ts`:
- Around line 49-60: The returned policy object blindly trusts the parsed
parameters and systemParameters which can be arbitrary YAML (causing downstream
schema traversal to fail); in the function that builds the return (where
variables parameters and systemParameters are set), validate that parameters is
a proper ParameterSchema (has a string "type" and an object "properties") and
that systemParameters is an object with the expected keys/shape, otherwise
replace with a safe default (e.g., { type: "object", properties: {} } for
parameters and {} or a typed-empty structure for systemParameters); perform
light runtime checks (typeof/Array.isArray and !== null) and coerce or fallback
before returning name/version/description/parameters/systemParameters.

In `@console/workspaces/pages/mcp-proxies/src/subComponents/AddMCPProxyForm.tsx`:
- Around line 127-141: Trim authValue before validation and when building the
request payload: compute a trimmedAuth = authValue.trim() (inside the
AddMCPProxyForm handler where hasValue is computed), use Boolean(trimmedAuth)
instead of Boolean(authValue) for hasValue, setAuthError based on trimmedAuth,
and set the body.auth.value to trimmedAuth (and apply the same change to the
other handler referenced around lines 211-230). This ensures whitespace-only
input is rejected and the request sends the trimmed credential.
- Line 20: Replace the direct MUI import of InputAdornment in
AddMCPProxyForm.tsx with the InputAdornment exported by `@wso2/oxygen-ui` (swap
the import for InputAdornment). Also tighten validation and payload for
authValue where Boolean(authValue) is used (references: authValue, authHeader,
and the form submission/validation logic in AddMCPProxyForm) by trimming the
string before checking/using it (e.g., use authValue.trim() for truthiness
checks and send value: authValue.trim() instead of the raw authValue) so
whitespace-only values are rejected just like authHeader.

In `@console/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsx`:
- Around line 482-490: The clickable policy row (the Box in ViewMCPProxy that
uses onClick to call setEditingPolicyIndex(index) and drag handlers like
onDragStart/onDrop/handleDropPolicy) is not keyboard-accessible; update the
element to be a semantic interactive control (e.g., use a button or an element
with role="button" and tabIndex={0}) and add keyboard handlers so Enter/Space
trigger setEditingPolicyIndex(index) and keyboard users can initiate drag/drop
(or expose equivalent move controls) and ensure appropriate ARIA attributes
(aria-pressed/aria-grabbed or aria-selected) and focus styles are present; apply
the same changes to the other instance referenced (the block around lines with
similar Box/Card handlers).
- Around line 1023-1043: The SVG in ViewMCPProxy (elements ellipse and several
path elements using fill/stroke attributes) uses hardcoded hex colors; update
these to use theme palette tokens via the component's sx props or use
currentColor with wrapper styles so colors respond to theme/high-contrast.
Replace literal fill/stroke values on the <ellipse> and each <path> with
references to theme tokens (e.g., palette.background, palette.surface,
palette.primary/neutral) through sx or by setting color on a wrapping element
and using fill="currentColor"/stroke="currentColor" where appropriate; ensure
strokeWidth/strokeLinejoin remain unchanged and that accessibility/contrast is
preserved.

---

Nitpick comments:
In `@console/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyTable.tsx`:
- Around line 300-320: The twoLetters and formatRelativeTime helper
implementations are duplicated and have diverged; extract both functions
(twoLetters and formatRelativeTime) into a shared utility module (e.g., a new
util file exported as named functions) and update MCPProxyTable.tsx to import
and use those shared helpers instead of the local definitions; ensure you
preserve the intended behavior (keep the current casing rules used in this file
or reconcile to the canonical behavior) and remove the duplicate local functions
so other components can import the same implementations to avoid UI drift.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 59ced317-cbb9-46e9-b34b-2e98aa10177c

📥 Commits

Reviewing files that changed from the base of the PR and between 8857b20 and 3bd19e4.

⛔ Files ignored due to path filters (1)
  • console/common/config/rush/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (74)
  • agent-manager-service/api/agent_mcp_proxy_routes.go
  • agent-manager-service/api/app.go
  • agent-manager-service/api/gateway_internal_routes.go
  • agent-manager-service/api/mcp_proxy_routes.go
  • agent-manager-service/controllers/agent_mcp_proxy_controller.go
  • agent-manager-service/controllers/gateway_internal_controller.go
  • agent-manager-service/controllers/mcp_proxy_controller.go
  • agent-manager-service/db_migrations/021_create_mcp_proxies.go
  • agent-manager-service/db_migrations/migration_list.go
  • agent-manager-service/models/agent_mcp_proxy_mapping.go
  • agent-manager-service/models/artifact.go
  • agent-manager-service/models/gateway_compat.go
  • agent-manager-service/models/mcp_proxy.go
  • agent-manager-service/repositories/agent_mcp_proxy_mapping_repository.go
  • agent-manager-service/repositories/artifact_repository.go
  • agent-manager-service/repositories/mcp_proxy_repository.go
  • agent-manager-service/services/agent_manager.go
  • agent-manager-service/services/agent_mcp_proxy_service.go
  • agent-manager-service/services/deployment_ack_handler.go
  • agent-manager-service/services/gateway_events_service.go
  • agent-manager-service/services/gateway_internal_service.go
  • agent-manager-service/services/mcp_proxy_deployment.go
  • agent-manager-service/services/mcp_proxy_service.go
  • agent-manager-service/services/platform_gateway_service.go
  • agent-manager-service/spec/mcp_proxies.go
  • agent-manager-service/utils/api.go
  • agent-manager-service/utils/errors.go
  • agent-manager-service/wiring/params.go
  • agent-manager-service/wiring/wire.go
  • agent-manager-service/wiring/wire_gen.go
  • console/apps/web-ui/vite.config.ts
  • console/rush.json
  • console/workspaces/core-ui/package.json
  • console/workspaces/core-ui/src/Layouts/OxygenLayout/navigationItems.tsx
  • console/workspaces/core-ui/src/Route/Route.tsx
  • console/workspaces/core-ui/src/pages/index.tsx
  • console/workspaces/core-ui/vite.config.ts
  • console/workspaces/libs/api-client/src/apis/agent-mcp-proxies.ts
  • console/workspaces/libs/api-client/src/apis/index.ts
  • console/workspaces/libs/api-client/src/apis/mcp-proxies.ts
  • console/workspaces/libs/api-client/src/hooks/agent-mcp-proxies.ts
  • console/workspaces/libs/api-client/src/hooks/index.ts
  • console/workspaces/libs/api-client/src/hooks/mcp-policies.ts
  • console/workspaces/libs/api-client/src/hooks/mcp-proxies.ts
  • console/workspaces/libs/types/src/api/agent-mcp-proxies.ts
  • console/workspaces/libs/types/src/api/index.ts
  • console/workspaces/libs/types/src/api/mcp-proxies.ts
  • console/workspaces/libs/types/src/routes/generated-route.map.ts
  • console/workspaces/libs/types/src/routes/routes.map.ts
  • console/workspaces/pages/configure-agent/package.json
  • console/workspaces/pages/configure-agent/src/AddMCPServer.Component.tsx
  • console/workspaces/pages/configure-agent/src/Configure.Component.tsx
  • console/workspaces/pages/configure-agent/src/Configure/subComponents/AgentMCPServersSection.tsx
  • console/workspaces/pages/configure-agent/src/ViewMCPServer.Component.tsx
  • console/workspaces/pages/configure-agent/src/index.ts
  • console/workspaces/pages/mcp-proxies/eslint.config.js
  • console/workspaces/pages/mcp-proxies/package.json
  • console/workspaces/pages/mcp-proxies/src/AddMCPProxy.Organization.tsx
  • console/workspaces/pages/mcp-proxies/src/MCPProxies.Organization.tsx
  • console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/FieldRenderers.tsx
  • console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/PolicyParameterEditor.tsx
  • console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/SchemaTree.tsx
  • console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/schemaUtils.ts
  • console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/types.ts
  • console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/yamlParser.ts
  • console/workspaces/pages/mcp-proxies/src/components/MCPCapabilitiesView.tsx
  • console/workspaces/pages/mcp-proxies/src/components/MCPLogo.tsx
  • console/workspaces/pages/mcp-proxies/src/index.ts
  • console/workspaces/pages/mcp-proxies/src/subComponents/AddMCPProxyForm.tsx
  • console/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyTable.tsx
  • console/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsx
  • console/workspaces/pages/mcp-proxies/tsconfig.json
  • console/workspaces/pages/mcp-proxies/tsconfig.lib.json
  • console/workspaces/pages/mcp-proxies/vite.config.ts

Comment thread agent-manager-service/controllers/gateway_internal_controller.go
Comment thread agent-manager-service/controllers/gateway_internal_controller.go
Comment thread agent-manager-service/controllers/mcp_proxy_controller.go
Comment thread agent-manager-service/controllers/mcp_proxy_controller.go
Comment thread agent-manager-service/models/agent_mcp_proxy_mapping.go
Comment thread console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/yamlParser.ts Outdated
Comment thread console/workspaces/pages/mcp-proxies/src/subComponents/AddMCPProxyForm.tsx Outdated
Comment thread console/workspaces/pages/mcp-proxies/src/subComponents/AddMCPProxyForm.tsx Outdated
Comment thread console/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsx Outdated
@Oshanath Oshanath force-pushed the feature-mcp-proxy branch 2 times, most recently from 3135a39 to 759dd10 Compare May 26, 2026 06:42
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

🧹 Nitpick comments (2)
agent-manager-service/services/gateway_internal_service.go (1)

147-149: 💤 Low value

Inconsistent error type in fallback path.

When deployment == nil in the fallback getActiveMCPDeploymentByArtifact, this returns ErrMCPProxyNotFound, but the primary path at line 126 returns ErrDeploymentNotActive for the same condition. Consider using ErrDeploymentNotActive here too for consistency, since a nil deployment means the proxy exists but isn't deployed.

Suggested fix
 	if deployment == nil {
-		return nil, utils.ErrMCPProxyNotFound
+		return nil, utils.ErrDeploymentNotActive
 	}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@agent-manager-service/services/gateway_internal_service.go` around lines 147
- 149, In getActiveMCPDeploymentByArtifact, the fallback branch currently
returns utils.ErrMCPProxyNotFound when deployment == nil while the primary path
returns utils.ErrDeploymentNotActive for the same nil-deployment condition;
change the fallback return to utils.ErrDeploymentNotActive so both paths are
consistent (update the branch that checks deployment == nil to return
utils.ErrDeploymentNotActive instead of utils.ErrMCPProxyNotFound).
agent-manager-service/services/mcp_proxy_deployment.go (1)

600-609: ⚖️ Poor tradeoff

Shallow clone may cause unintended mutation of nested structures.

cloneStringInterfaceMap performs a shallow copy, so nested map[string]interface{} or slice values remain shared with the original. In mergeMCPPolicyParams, when you later call mergeMCPPolicyParamValue which recursively merges maps, modifications to nested structures could inadvertently mutate the original base parameter.

Consider deep-cloning nested maps, or ensure callers never mutate the returned map in-place.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@agent-manager-service/services/mcp_proxy_deployment.go` around lines 600 -
609, cloneStringInterfaceMap currently does a shallow copy which leaves nested
map[string]interface{} and []interface{} values shared and can be mutated later
by mergeMCPPolicyParamValue/mergeMCPPolicyParams; change cloneStringInterfaceMap
to perform a deep clone: when a value is a map[string]interface{} recursively
deep-copy it, when it is a []interface{} deep-copy each element (recursing for
nested maps/slices), and for primitive/other types copy by value, so the
returned map is independent of the input and safe to modify in
mergeMCPPolicyParamValue/mergeMCPPolicyParams; alternatively, document and
enforce that callers never mutate the returned map in-place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@agent-manager-service/controllers/agent_mcp_proxy_controller.go`:
- Around line 117-118: The handler currently reads
r.PathValue(utils.PathParamProxyId) into mappingName which blurs the contract
between proxyId and mappingName; change the path param usage so the route and
handler consistently use a mappingName identifier: update the controller to call
r.PathValue(utils.PathParamMappingName) (or a new utils.PathParamMappingName
constant) wherever mappingName is expected (including the other occurrences
around lines 151-152), and update the route placeholders in
agent_mcp_proxy_routes.go from `{proxyId}` to `{mappingName}` to keep path
semantics aligned.

In `@agent-manager-service/repositories/mcp_proxy_repository.go`:
- Around line 88-104: The MCP proxy update currently ignores RowsAffected and
can silently succeed when no proxy row is matched; after performing the
tx.Model(&models.MCPProxy{})...Updates call, check the result.RowsAffected and
if it equals 0 return gorm.ErrRecordNotFound (or wrap/return that error) instead
of continuing; only call r.artifactRepo.Update(tx, &models.Artifact{...}) when
the proxy update actually affected at least one row.

In `@agent-manager-service/services/agent_mcp_proxy_service.go`:
- Around line 186-190: The broadcast for artifact deletion is being called
before the DB delete completes (see s.mcpSvc.BroadcastMCPArtifactDeletion and
s.repo.Delete), so move the broadcast calls to after you confirm Delete
succeeded: call s.repo.Delete(agentName, projectName, orgName, name), check err
is nil (and not gorm.ErrRecordNotFound), then, if mapping != nil &&
mapping.ArtifactUUID != nil and s.mcpSvc != nil, call
s.mcpSvc.BroadcastMCPArtifactDeletion(*mapping.ArtifactUUID, orgName); apply the
same change for the other broadcast block (lines 214-219) so broadcasts only run
after a successful repository delete.

In `@console/workspaces/libs/api-client/src/hooks/mcp-policies.ts`:
- Around line 44-49: getMCPPoliciesCatalogUrl currently calls new URL(...)
synchronously and can throw during render; wrap the URL parsing in a try/catch
inside getMCPPoliciesCatalogUrl (around the new
URL(globalConfig.guardrailsCatalogUrl) call) and on any error return an empty
string so useMCPPoliciesCatalog's enabled: Boolean(url && orgName) will disable
the query; preserve the existing logic that sets searchParams ("categories" and
default "limit") and only call toString() when parsing succeeds.

In `@console/workspaces/pages/mcp-proxies/package.json`:
- Around line 11-14: package.json currently uses "type": "module" but the
exports["."].require condition points to ./dist/index.js which Node will treat
as ESM and cause ERR_REQUIRE_ESM for CJS consumers; fix by removing the
exports.require condition (or change it to point to a real CommonJS build like
./dist/index.cjs if you publish one) and ensure exports.types/import map remain
unchanged; update the exports object (the "." entry) so it only lists "import"
and "types" when no CJS artifact is provided.

In
`@console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/PolicyParameterEditor.tsx`:
- Around line 261-265: The effect in PolicyParameterEditor only updates state
when existingValues is truthy, so when existingValues becomes undefined the
previous values remain; change the useEffect to always call setValues with
initializeDefaultValues using the current parameters and existingValues (i.e.,
call setValues(initializeDefaultValues(parameters, existingValues)) even when
existingValues is undefined) so the form is reset when existingValues is
cleared; update the effect that references existingValues, parameters,
setValues, and initializeDefaultValues accordingly.

In `@console/workspaces/pages/mcp-proxies/src/subComponents/AddMCPProxyForm.tsx`:
- Around line 323-326: The Chip label in AddMCPProxyForm currently always
prepends "v" to serverVersion which causes double "v" when serverVersion already
starts with "v"; change the label construction on the Chip (the label prop where
serverVersion is used) to conditionally prefix only when needed, e.g. use
serverVersion.startsWith('v') ? serverVersion : `v${serverVersion}` (or strip a
leading 'v' before prefixing) so values like "v1.0" render as "v1.0" and "1.0"
render as "v1.0".

In `@console/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyTable.tsx`:
- Around line 309-312: formatRelativeTime currently returns an empty string for
invalid timestamps which leaves the “Last Updated” cell blank; change
formatRelativeTime(value: string) so that when new Date(value) is invalid it
returns a visible fallback (e.g. "—" or "Unknown") instead of "" and ensure any
places that render its output can treat that fallback as a displayable string;
update the function body where Number.isNaN(timestamp) is checked to return the
chosen visible fallback.

In `@console/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsx`:
- Around line 110-134: The effect unconditionally resets all draft fields and
policies whenever fresh proxy props arrive, wiping unsaved edits; change the
effect (the useEffect that reads proxy?.id / proxy?.policies / proxy?.name /
proxy?.version / proxy?.context / proxy?.description) to only overwrite local
state when either the proxy id actually changed (proxy?.id differs from
baselineDetails.id) or when the user is not currently editing (isEditingDetails
is false and editablePolicies equals baselinePolicies), otherwise preserve
editablePolicies, name, version, context and description; update
baselinePolicies and baselineDetails only when you accept the server update (on
id change or when not editing) and keep existing setEditablePolicies,
setBaselinePolicies, setName, setVersion, setContext, setDescription,
setBaselineDetails, setIsEditingDetails calls guarded by that condition.

---

Nitpick comments:
In `@agent-manager-service/services/gateway_internal_service.go`:
- Around line 147-149: In getActiveMCPDeploymentByArtifact, the fallback branch
currently returns utils.ErrMCPProxyNotFound when deployment == nil while the
primary path returns utils.ErrDeploymentNotActive for the same nil-deployment
condition; change the fallback return to utils.ErrDeploymentNotActive so both
paths are consistent (update the branch that checks deployment == nil to return
utils.ErrDeploymentNotActive instead of utils.ErrMCPProxyNotFound).

In `@agent-manager-service/services/mcp_proxy_deployment.go`:
- Around line 600-609: cloneStringInterfaceMap currently does a shallow copy
which leaves nested map[string]interface{} and []interface{} values shared and
can be mutated later by mergeMCPPolicyParamValue/mergeMCPPolicyParams; change
cloneStringInterfaceMap to perform a deep clone: when a value is a
map[string]interface{} recursively deep-copy it, when it is a []interface{}
deep-copy each element (recursing for nested maps/slices), and for
primitive/other types copy by value, so the returned map is independent of the
input and safe to modify in mergeMCPPolicyParamValue/mergeMCPPolicyParams;
alternatively, document and enforce that callers never mutate the returned map
in-place.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0772c1cd-647e-4491-920a-e8dbfc9cc599

📥 Commits

Reviewing files that changed from the base of the PR and between 3bd19e4 and 3135a39.

⛔ Files ignored due to path filters (1)
  • console/common/config/rush/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (82)
  • agent-manager-service/api/agent_mcp_proxy_routes.go
  • agent-manager-service/api/app.go
  • agent-manager-service/api/gateway_internal_routes.go
  • agent-manager-service/api/mcp_proxy_routes.go
  • agent-manager-service/controllers/agent_mcp_proxy_controller.go
  • agent-manager-service/controllers/gateway_internal_controller.go
  • agent-manager-service/controllers/mcp_proxy_controller.go
  • agent-manager-service/db_migrations/021_create_mcp_proxies.go
  • agent-manager-service/db_migrations/migration_list.go
  • agent-manager-service/models/agent_mcp_proxy_mapping.go
  • agent-manager-service/models/artifact.go
  • agent-manager-service/models/gateway_compat.go
  • agent-manager-service/models/mcp_proxy.go
  • agent-manager-service/repositories/agent_mcp_proxy_mapping_repository.go
  • agent-manager-service/repositories/artifact_repository.go
  • agent-manager-service/repositories/mcp_proxy_repository.go
  • agent-manager-service/services/agent_manager.go
  • agent-manager-service/services/agent_mcp_proxy_service.go
  • agent-manager-service/services/deployment_ack_handler.go
  • agent-manager-service/services/gateway_events_service.go
  • agent-manager-service/services/gateway_internal_service.go
  • agent-manager-service/services/mcp_proxy_deployment.go
  • agent-manager-service/services/mcp_proxy_service.go
  • agent-manager-service/services/platform_gateway_service.go
  • agent-manager-service/utils/api.go
  • agent-manager-service/utils/errors.go
  • agent-manager-service/wiring/params.go
  • agent-manager-service/wiring/wire.go
  • agent-manager-service/wiring/wire_gen.go
  • console/apps/web-ui/vite.config.ts
  • console/rush.json
  • console/workspaces/core-ui/package.json
  • console/workspaces/core-ui/src/Layouts/OxygenLayout/navigationItems.tsx
  • console/workspaces/core-ui/src/Route/Route.tsx
  • console/workspaces/core-ui/src/pages/index.tsx
  • console/workspaces/core-ui/vite.config.ts
  • console/workspaces/libs/api-client/src/apis/agent-mcp-proxies.ts
  • console/workspaces/libs/api-client/src/apis/index.ts
  • console/workspaces/libs/api-client/src/apis/mcp-proxies.ts
  • console/workspaces/libs/api-client/src/hooks/agent-mcp-proxies.ts
  • console/workspaces/libs/api-client/src/hooks/index.ts
  • console/workspaces/libs/api-client/src/hooks/mcp-policies.ts
  • console/workspaces/libs/api-client/src/hooks/mcp-proxies.ts
  • console/workspaces/libs/shared-component/package.json
  • console/workspaces/libs/shared-component/src/index.ts
  • console/workspaces/libs/shared-component/src/utils/policyParameterEditor/index.ts
  • console/workspaces/libs/shared-component/src/utils/policyParameterEditor/schemaUtils.ts
  • console/workspaces/libs/shared-component/src/utils/policyParameterEditor/types.ts
  • console/workspaces/libs/shared-component/src/utils/policyParameterEditor/yamlParser.ts
  • console/workspaces/libs/types/src/api/agent-mcp-proxies.ts
  • console/workspaces/libs/types/src/api/index.ts
  • console/workspaces/libs/types/src/api/mcp-proxies.ts
  • console/workspaces/libs/types/src/routes/generated-route.map.ts
  • console/workspaces/libs/types/src/routes/routes.map.ts
  • console/workspaces/pages/configure-agent/package.json
  • console/workspaces/pages/configure-agent/src/AddMCPServer.Component.tsx
  • console/workspaces/pages/configure-agent/src/Configure.Component.tsx
  • console/workspaces/pages/configure-agent/src/Configure/subComponents/AgentMCPServersSection.tsx
  • console/workspaces/pages/configure-agent/src/ViewMCPServer.Component.tsx
  • console/workspaces/pages/configure-agent/src/index.ts
  • console/workspaces/pages/llm-providers/src/PolicyParameterEditor/FieldRenderers.tsx
  • console/workspaces/pages/llm-providers/src/PolicyParameterEditor/PolicyParameterEditor.tsx
  • console/workspaces/pages/llm-providers/src/PolicyParameterEditor/SchemaTree.tsx
  • console/workspaces/pages/llm-providers/src/components/GuardrailSelectorDrawer/GuardrailSelectorDrawer.tsx
  • console/workspaces/pages/llm-providers/src/subComponents/GuardrailsSection.tsx
  • console/workspaces/pages/llm-providers/src/subComponents/LLMProviderGuardrailsTab.tsx
  • console/workspaces/pages/mcp-proxies/eslint.config.js
  • console/workspaces/pages/mcp-proxies/package.json
  • console/workspaces/pages/mcp-proxies/src/AddMCPProxy.Organization.tsx
  • console/workspaces/pages/mcp-proxies/src/MCPProxies.Organization.tsx
  • console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/FieldRenderers.tsx
  • console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/PolicyParameterEditor.tsx
  • console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/SchemaTree.tsx
  • console/workspaces/pages/mcp-proxies/src/components/MCPCapabilitiesView.tsx
  • console/workspaces/pages/mcp-proxies/src/components/MCPLogo.tsx
  • console/workspaces/pages/mcp-proxies/src/index.ts
  • console/workspaces/pages/mcp-proxies/src/subComponents/AddMCPProxyForm.tsx
  • console/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyTable.tsx
  • console/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsx
  • console/workspaces/pages/mcp-proxies/tsconfig.json
  • console/workspaces/pages/mcp-proxies/tsconfig.lib.json
  • console/workspaces/pages/mcp-proxies/vite.config.ts
✅ Files skipped from review due to trivial changes (12)
  • console/workspaces/pages/llm-providers/src/subComponents/GuardrailsSection.tsx
  • console/workspaces/pages/configure-agent/src/index.ts
  • console/workspaces/libs/shared-component/src/index.ts
  • agent-manager-service/models/artifact.go
  • console/workspaces/libs/shared-component/src/utils/policyParameterEditor/index.ts
  • agent-manager-service/db_migrations/migration_list.go
  • agent-manager-service/services/gateway_events_service.go
  • console/workspaces/pages/llm-providers/src/PolicyParameterEditor/SchemaTree.tsx
  • console/workspaces/pages/configure-agent/src/Configure.Component.tsx
  • console/rush.json
  • agent-manager-service/wiring/wire_gen.go
  • console/workspaces/libs/api-client/src/apis/index.ts

Comment thread agent-manager-service/controllers/agent_mcp_proxy_controller.go Outdated
Comment thread agent-manager-service/repositories/mcp_proxy_repository.go Outdated
Comment thread agent-manager-service/services/agent_mcp_proxy_service.go
Comment thread console/workspaces/libs/api-client/src/hooks/mcp-policies.ts Outdated
Comment thread console/workspaces/pages/mcp-proxies/package.json
@Oshanath Oshanath force-pushed the feature-mcp-proxy branch 2 times, most recently from 97370fd to fb917d2 Compare May 26, 2026 07:27
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (4)
agent-manager-service/api/agent_mcp_proxy_routes.go (1)

37-42: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Align binding identifier naming between route and handler.

Line 37 and Line 41 use {proxyId}, but update/delete handlers treat this value as a mapping name. This makes the API contract ambiguous and can lead clients to send an MCP proxy UUID for operations that target a binding identifier.

💡 Suggested minimal fix
- "PUT /orgs/{orgName}/projects/{projName}/agents/{agentName}/mcp-proxies/{proxyId}",
+ "PUT /orgs/{orgName}/projects/{projName}/agents/{agentName}/mcp-proxies/{mappingName}",

- "DELETE /orgs/{orgName}/projects/{projName}/agents/{agentName}/mcp-proxies/{proxyId}",
+ "DELETE /orgs/{orgName}/projects/{projName}/agents/{agentName}/mcp-proxies/{mappingName}",

And in controller extraction, use a matching path param constant/key for mappingName.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@agent-manager-service/api/agent_mcp_proxy_routes.go` around lines 37 - 42,
Routes use the path parameter name "{proxyId}" but the handlers
UpdateAgentMCPProxy and RemoveAgentMCPProxy expect a binding/mapping identifier,
causing a naming mismatch; rename the route path parameters (both PUT and
DELETE) from "{proxyId}" to "{mappingName}" and update the controller extraction
code to read the same key (use the mappingName constant/key) so the route and
handler use the identical identifier name for the mapping binding.
agent-manager-service/services/agent_mcp_proxy_service.go (1)

208-219: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

deletedAny flag set before confirming delete success; broadcast also precedes delete.

At line 213, deletedAny = true is set before the delete at line 217 actually succeeds. If all deletes fail, injectMCPEnvVars will still be called. Additionally, the broadcast at lines 214-216 fires before the corresponding delete, same issue as Remove.

Proposed fix
 	for _, mapping := range mappings {
 		if mapping == nil {
 			continue
 		}
-		deletedAny = true
-		if s.mcpSvc != nil && mapping.ArtifactUUID != nil {
-			s.mcpSvc.BroadcastMCPArtifactDeletion(*mapping.ArtifactUUID, orgName)
-		}
 		if _, err := s.repo.Delete(agentName, projectName, orgName, mapping.Name); err != nil {
 			errs = append(errs, fmt.Errorf("delete binding %q: %w", mapping.Name, err))
+			continue
+		}
+		deletedAny = true
+		if s.mcpSvc != nil && mapping.ArtifactUUID != nil {
+			s.mcpSvc.BroadcastMCPArtifactDeletion(*mapping.ArtifactUUID, orgName)
 		}
 	}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@agent-manager-service/services/agent_mcp_proxy_service.go` around lines 208 -
219, The deletedAny flag is being set and BroadcastMCPArtifactDeletion is called
before the actual delete is confirmed; update the loop in the method so that you
call s.repo.Delete(agentName, projectName, orgName, mapping.Name) first, check
err==nil, only then set deletedAny = true and, if mapping.ArtifactUUID != nil
and s.mcpSvc != nil, call
s.mcpSvc.BroadcastMCPArtifactDeletion(*mapping.ArtifactUUID, orgName); this
ensures deletedAny only becomes true on successful deletion and broadcasts occur
after a successful delete.
console/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsx (2)

907-916: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make capability cards keyboard-accessible.

Line 907 uses clickable Card with mouse-only interaction. Add keyboard operability (Enter/Space) so users can select/deselect items without a pointer.

♿ Suggested accessibility fix
               <Card
                 key={itemKey}
                 variant="outlined"
                 onClick={() => setSelectedItemKey(selected ? "" : itemKey)}
+                role="button"
+                tabIndex={0}
+                onKeyDown={(event) => {
+                  if (event.key === "Enter" || event.key === " ") {
+                    event.preventDefault();
+                    setSelectedItemKey(selected ? "" : itemKey);
+                  }
+                }}
                 sx={{
                   borderRadius: 1,
                   cursor: "pointer",
                   p: 1.5,
                 }}
               >
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@console/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsx`
around lines 907 - 916, The Card currently used as a clickable-only element
needs keyboard operability: make the Card focusable (add tabIndex=0) and expose
a proper role (e.g., role="button"), and add an onKeyDown handler that calls the
same toggle logic as the onClick (invoke setSelectedItemKey(selected ? "" :
itemKey) when Enter or Space is pressed). Alternatively, replace the Card body
with a focusable action component (e.g., CardActionArea) that delegates to the
same setSelectedItemKey logic; ensure you reference the existing
setSelectedItemKey, selected, and itemKey identifiers so the keyboard handler
and click handler remain consistent.

110-134: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Prevent incoming proxy refreshes from wiping local drafts.

Line 110 unconditionally rehydrates details/policies and exits edit mode. If fresh data arrives during editing, unsaved changes are lost.

💡 Minimal guard to preserve unsaved edits for the same proxy
-import { useEffect, useMemo, useState } from "react";
+import { useEffect, useMemo, useRef, useState } from "react";
...
+  const lastLoadedProxyIdRef = useRef<string | undefined>(undefined);
...
   useEffect(() => {
+    if (!proxy?.id) return;
+    const isSameProxy = lastLoadedProxyIdRef.current === proxy.id;
+    if (isSameProxy && hasUnsavedChanges) return;
+    lastLoadedProxyIdRef.current = proxy.id;
+
     const nextPolicies = proxy?.policies ?? [];
     setEditablePolicies(nextPolicies);
     setBaselinePolicies(nextPolicies);
...
-  }, [
-    proxy?.id,
-    proxy?.policies,
-    proxy?.name,
-    proxy?.version,
-    proxy?.context,
-    proxy?.description,
-  ]);
+  }, [proxy, hasUnsavedChanges]);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@console/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsx`
around lines 110 - 134, The effect currently always rehydrates fields and exits
edit mode, which wipes local drafts; change the useEffect to skip overwriting
when the user is editing the same proxy: add a guard that if isEditingDetails is
true and proxy?.id equals the stored baseline proxy id (or include id in
baselineDetails) then return early and do not call
setEditablePolicies/setBaselinePolicies/setName/setVersion/setContext/setDescription/setBaselineDetails/setIsEditingDetails;
only rehydrate when not editing or when proxy?.id has changed (in which case
update the baseline id/state and setIsEditingDetails(false)). Ensure you either
include the proxy id in baselineDetails or add a new state like baselineProxyId
and update it in the same effect so comparisons can be made.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@agent-manager-service/services/agent_manager.go`:
- Around line 2161-2172: The current flow calls buildMCPServerURLsForAgent
(mcpURLs) and unconditionally removes any incoming MCP_SERVER_URLS earlier, but
buildMCPServerURLsForAgent hides lookup errors by returning nil which can cause
valid MCP_SERVER_URLS to be cleared on transient DB/service failures; change
buildMCPServerURLsForAgent to return ([]string, error) and update the caller in
the deploy path to only overwrite deployReq.Env (mcpServerURLsEnvKey) when err
== nil and len(mcpURLs) > 0, otherwise log the error and preserve any existing
MCP_SERVER_URLS in deployReq.Env; apply the same pattern to the other call site
that uses buildMCPServerURLsForAgent so failed rebuilds never silently clear
prior bindings.

In `@console/apps/web-ui/public/config.js`:
- Line 36: The value for disableAuth in config.js is using a self-compare
('true' === 'true') so it is always true; change it to evaluate a real
configuration source instead (for example, read the runtime/build config or an
environment variable named DISABLE_AUTH) and set disableAuth to a boolean based
on that value rather than a hardcoded self-comparison; update the assignment for
disableAuth in public/config.js (the disableAuth symbol) to derive its boolean
from the actual config source.

In
`@console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/FieldRenderers.tsx`:
- Around line 295-300: For numeric arrays (itemType === "number" || itemType ===
"integer") ensure you convert and validate each tag before calling onChange: for
each value in newVal attempt Number(v) and only include it if it's not NaN;
additionally if itemType === "integer" require Number.isInteger(parsed)
(reject/omit values with decimals). Replace the current parsed logic in
FieldRenderers.tsx (the parsed variable tied to newVal) with a conversion+filter
that yields an array of strictly valid numbers, then call onChange(node.path,
validNumbers); consider setting a validation error when any tag is invalid
instead of silently dropping values.

---

Duplicate comments:
In `@agent-manager-service/api/agent_mcp_proxy_routes.go`:
- Around line 37-42: Routes use the path parameter name "{proxyId}" but the
handlers UpdateAgentMCPProxy and RemoveAgentMCPProxy expect a binding/mapping
identifier, causing a naming mismatch; rename the route path parameters (both
PUT and DELETE) from "{proxyId}" to "{mappingName}" and update the controller
extraction code to read the same key (use the mappingName constant/key) so the
route and handler use the identical identifier name for the mapping binding.

In `@agent-manager-service/services/agent_mcp_proxy_service.go`:
- Around line 208-219: The deletedAny flag is being set and
BroadcastMCPArtifactDeletion is called before the actual delete is confirmed;
update the loop in the method so that you call s.repo.Delete(agentName,
projectName, orgName, mapping.Name) first, check err==nil, only then set
deletedAny = true and, if mapping.ArtifactUUID != nil and s.mcpSvc != nil, call
s.mcpSvc.BroadcastMCPArtifactDeletion(*mapping.ArtifactUUID, orgName); this
ensures deletedAny only becomes true on successful deletion and broadcasts occur
after a successful delete.

In `@console/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsx`:
- Around line 907-916: The Card currently used as a clickable-only element needs
keyboard operability: make the Card focusable (add tabIndex=0) and expose a
proper role (e.g., role="button"), and add an onKeyDown handler that calls the
same toggle logic as the onClick (invoke setSelectedItemKey(selected ? "" :
itemKey) when Enter or Space is pressed). Alternatively, replace the Card body
with a focusable action component (e.g., CardActionArea) that delegates to the
same setSelectedItemKey logic; ensure you reference the existing
setSelectedItemKey, selected, and itemKey identifiers so the keyboard handler
and click handler remain consistent.
- Around line 110-134: The effect currently always rehydrates fields and exits
edit mode, which wipes local drafts; change the useEffect to skip overwriting
when the user is editing the same proxy: add a guard that if isEditingDetails is
true and proxy?.id equals the stored baseline proxy id (or include id in
baselineDetails) then return early and do not call
setEditablePolicies/setBaselinePolicies/setName/setVersion/setContext/setDescription/setBaselineDetails/setIsEditingDetails;
only rehydrate when not editing or when proxy?.id has changed (in which case
update the baseline id/state and setIsEditingDetails(false)). Ensure you either
include the proxy id in baselineDetails or add a new state like baselineProxyId
and update it in the same effect so comparisons can be made.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0dd38280-8674-44fe-a0ae-7347cfc6ea05

📥 Commits

Reviewing files that changed from the base of the PR and between 3135a39 and b523cbf.

⛔ Files ignored due to path filters (1)
  • console/common/config/rush/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (83)
  • agent-manager-service/api/agent_mcp_proxy_routes.go
  • agent-manager-service/api/app.go
  • agent-manager-service/api/gateway_internal_routes.go
  • agent-manager-service/api/mcp_proxy_routes.go
  • agent-manager-service/controllers/agent_mcp_proxy_controller.go
  • agent-manager-service/controllers/gateway_internal_controller.go
  • agent-manager-service/controllers/mcp_proxy_controller.go
  • agent-manager-service/db_migrations/023_create_mcp_proxies.go
  • agent-manager-service/db_migrations/migration_list.go
  • agent-manager-service/models/agent_mcp_proxy_mapping.go
  • agent-manager-service/models/artifact.go
  • agent-manager-service/models/gateway_compat.go
  • agent-manager-service/models/mcp_proxy.go
  • agent-manager-service/repositories/agent_mcp_proxy_mapping_repository.go
  • agent-manager-service/repositories/artifact_repository.go
  • agent-manager-service/repositories/mcp_proxy_repository.go
  • agent-manager-service/services/agent_manager.go
  • agent-manager-service/services/agent_mcp_proxy_service.go
  • agent-manager-service/services/deployment_ack_handler.go
  • agent-manager-service/services/gateway_events_service.go
  • agent-manager-service/services/gateway_internal_service.go
  • agent-manager-service/services/mcp_proxy_deployment.go
  • agent-manager-service/services/mcp_proxy_service.go
  • agent-manager-service/services/platform_gateway_service.go
  • agent-manager-service/utils/api.go
  • agent-manager-service/utils/errors.go
  • agent-manager-service/wiring/params.go
  • agent-manager-service/wiring/wire.go
  • agent-manager-service/wiring/wire_gen.go
  • console/apps/web-ui/public/config.js
  • console/apps/web-ui/vite.config.ts
  • console/rush.json
  • console/workspaces/core-ui/package.json
  • console/workspaces/core-ui/src/Layouts/OxygenLayout/navigationItems.tsx
  • console/workspaces/core-ui/src/Route/Route.tsx
  • console/workspaces/core-ui/src/pages/index.tsx
  • console/workspaces/core-ui/vite.config.ts
  • console/workspaces/libs/api-client/src/apis/agent-mcp-proxies.ts
  • console/workspaces/libs/api-client/src/apis/index.ts
  • console/workspaces/libs/api-client/src/apis/mcp-proxies.ts
  • console/workspaces/libs/api-client/src/hooks/agent-mcp-proxies.ts
  • console/workspaces/libs/api-client/src/hooks/index.ts
  • console/workspaces/libs/api-client/src/hooks/mcp-policies.ts
  • console/workspaces/libs/api-client/src/hooks/mcp-proxies.ts
  • console/workspaces/libs/shared-component/package.json
  • console/workspaces/libs/shared-component/src/index.ts
  • console/workspaces/libs/shared-component/src/utils/policyParameterEditor/index.ts
  • console/workspaces/libs/shared-component/src/utils/policyParameterEditor/schemaUtils.ts
  • console/workspaces/libs/shared-component/src/utils/policyParameterEditor/types.ts
  • console/workspaces/libs/shared-component/src/utils/policyParameterEditor/yamlParser.ts
  • console/workspaces/libs/types/src/api/agent-mcp-proxies.ts
  • console/workspaces/libs/types/src/api/index.ts
  • console/workspaces/libs/types/src/api/mcp-proxies.ts
  • console/workspaces/libs/types/src/routes/generated-route.map.ts
  • console/workspaces/libs/types/src/routes/routes.map.ts
  • console/workspaces/pages/configure-agent/package.json
  • console/workspaces/pages/configure-agent/src/AddMCPServer.Component.tsx
  • console/workspaces/pages/configure-agent/src/Configure.Component.tsx
  • console/workspaces/pages/configure-agent/src/Configure/subComponents/AgentMCPServersSection.tsx
  • console/workspaces/pages/configure-agent/src/ViewMCPServer.Component.tsx
  • console/workspaces/pages/configure-agent/src/index.ts
  • console/workspaces/pages/llm-providers/src/PolicyParameterEditor/FieldRenderers.tsx
  • console/workspaces/pages/llm-providers/src/PolicyParameterEditor/PolicyParameterEditor.tsx
  • console/workspaces/pages/llm-providers/src/PolicyParameterEditor/SchemaTree.tsx
  • console/workspaces/pages/llm-providers/src/components/GuardrailSelectorDrawer/GuardrailSelectorDrawer.tsx
  • console/workspaces/pages/llm-providers/src/subComponents/GuardrailsSection.tsx
  • console/workspaces/pages/llm-providers/src/subComponents/LLMProviderGuardrailsTab.tsx
  • console/workspaces/pages/mcp-proxies/eslint.config.js
  • console/workspaces/pages/mcp-proxies/package.json
  • console/workspaces/pages/mcp-proxies/src/AddMCPProxy.Organization.tsx
  • console/workspaces/pages/mcp-proxies/src/MCPProxies.Organization.tsx
  • console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/FieldRenderers.tsx
  • console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/PolicyParameterEditor.tsx
  • console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/SchemaTree.tsx
  • console/workspaces/pages/mcp-proxies/src/components/MCPCapabilitiesView.tsx
  • console/workspaces/pages/mcp-proxies/src/components/MCPLogo.tsx
  • console/workspaces/pages/mcp-proxies/src/index.ts
  • console/workspaces/pages/mcp-proxies/src/subComponents/AddMCPProxyForm.tsx
  • console/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyTable.tsx
  • console/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsx
  • console/workspaces/pages/mcp-proxies/tsconfig.json
  • console/workspaces/pages/mcp-proxies/tsconfig.lib.json
  • console/workspaces/pages/mcp-proxies/vite.config.ts
✅ Files skipped from review due to trivial changes (6)
  • console/workspaces/libs/shared-component/package.json
  • agent-manager-service/models/artifact.go
  • console/workspaces/pages/llm-providers/src/PolicyParameterEditor/FieldRenderers.tsx
  • console/workspaces/pages/mcp-proxies/tsconfig.lib.json
  • console/workspaces/libs/types/src/api/mcp-proxies.ts
  • agent-manager-service/wiring/wire_gen.go

Comment thread agent-manager-service/services/agent_manager.go
Comment thread console/apps/web-ui/public/config.js Outdated
Comment thread console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/FieldRenderers.tsx Outdated
@Oshanath Oshanath force-pushed the feature-mcp-proxy branch from b523cbf to 7899ae6 Compare May 26, 2026 10:22
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 8

♻️ Duplicate comments (2)
agent-manager-service/services/agent_mcp_proxy_service.go (1)

186-190: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Broadcast deletion events only after successful DB delete.

BroadcastMCPArtifactDeletion is called before repo.Delete. If delete fails, downstream undeploy/deletion can run while the mapping still exists.

Proposed fix
-	if s.mcpSvc != nil && mapping != nil && mapping.ArtifactUUID != nil {
-		s.mcpSvc.BroadcastMCPArtifactDeletion(*mapping.ArtifactUUID, orgName)
-	}
 	if _, err := s.repo.Delete(agentName, projectName, orgName, name); err != nil {
 		if errors.Is(err, gorm.ErrRecordNotFound) {
 			return utils.ErrMCPProxyNotFound
 		}
 		return fmt.Errorf("delete binding: %w", err)
 	}
+	if s.mcpSvc != nil && mapping != nil && mapping.ArtifactUUID != nil {
+		s.mcpSvc.BroadcastMCPArtifactDeletion(*mapping.ArtifactUUID, orgName)
+	}
-		if s.mcpSvc != nil && mapping.ArtifactUUID != nil {
-			s.mcpSvc.BroadcastMCPArtifactDeletion(*mapping.ArtifactUUID, orgName)
-		}
 		if _, err := s.repo.Delete(agentName, projectName, orgName, mapping.Name); err != nil {
 			errs = append(errs, fmt.Errorf("delete binding %q: %w", mapping.Name, err))
+			continue
+		}
+		if s.mcpSvc != nil && mapping.ArtifactUUID != nil {
+			s.mcpSvc.BroadcastMCPArtifactDeletion(*mapping.ArtifactUUID, orgName)
 		}

Also applies to: 214-219

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@agent-manager-service/services/agent_mcp_proxy_service.go` around lines 186 -
190, The code currently calls
s.mcpSvc.BroadcastMCPArtifactDeletion(*mapping.ArtifactUUID, orgName) before
removing the DB mapping via s.repo.Delete, which can trigger downstream removal
while the mapping still exists; move the BroadcastMCPArtifactDeletion call so it
only executes after s.repo.Delete returns no error (and mapping/ArtifactUUID are
non-nil), updating both places where this pattern appears (the block using
mapping.ArtifactUUID and the similar block later) to first perform delete, check
errors.Is(err, gorm.ErrRecordNotFound) / other errors, and only then call
s.mcpSvc.BroadcastMCPArtifactDeletion with agentName, projectName, orgName, name
and the dereferenced ArtifactUUID.
console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/PolicyParameterEditor.tsx (1)

261-263: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Reset validation errors when reinitializing form values.

values are reinitialized when existingValues/parameters change, but errors are left as-is. This can show stale errors from the previous policy/edit session.

Suggested minimal fix
   useEffect(() => {
     setValues(initializeDefaultValues(parameters, existingValues));
+    setErrors({});
   }, [existingValues, parameters]);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/PolicyParameterEditor.tsx`
around lines 261 - 263, When reinitializing form values in the useEffect that
calls setValues(initializeDefaultValues(parameters, existingValues)), also reset
the validation state to avoid stale errors; update the same useEffect (or the
initialize flow) to call setErrors(...) with an empty errors object or freshly
computed errors (e.g., via an initializeErrors helper) after setValues. Target
the useEffect that references initializeDefaultValues, the setValues state
setter, and the setErrors state setter to implement this change.
🧹 Nitpick comments (1)
agent-manager-service/db_migrations/023_create_mcp_proxies.go (1)

38-56: ⚡ Quick win

Add indexes for foreign-key columns in agent_mcp_proxy_mappings.

mcp_proxy_uuid and artifact_uuid are foreign keys but currently unindexed; FK checks/cascades and related lookups can degrade to table scans as data grows.

🛠️ Proposed change
 			CREATE TABLE agent_mcp_proxy_mappings (
 				id SERIAL PRIMARY KEY,
 				agent_id VARCHAR(255) NOT NULL,
 				project_name VARCHAR(255) NOT NULL,
 				org_name VARCHAR(255) NOT NULL,
 				name VARCHAR(255) NOT NULL,
 				description TEXT,
 				mcp_proxy_uuid UUID NOT NULL,
 				artifact_uuid UUID,
 				policy_configuration JSONB NOT NULL DEFAULT '[]'::jsonb,
 				created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,

 				CONSTRAINT fk_agent_mcp_proxy_mapping_proxy FOREIGN KEY (mcp_proxy_uuid)
 					REFERENCES mcp_proxies(uuid) ON DELETE CASCADE,
 				CONSTRAINT fk_agent_mcp_proxy_mapping_artifact FOREIGN KEY (artifact_uuid)
 					REFERENCES artifacts(uuid) ON DELETE SET NULL,
 				CONSTRAINT uq_agent_mcp_proxy_name
 					UNIQUE (agent_id, project_name, org_name, name)
 			);
+			CREATE INDEX idx_agent_mcp_proxy_mappings_mcp_proxy_uuid
+				ON agent_mcp_proxy_mappings (mcp_proxy_uuid);
+			CREATE INDEX idx_agent_mcp_proxy_mappings_artifact_uuid
+				ON agent_mcp_proxy_mappings (artifact_uuid);
 		`
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@agent-manager-service/db_migrations/023_create_mcp_proxies.go` around lines
38 - 56, The migration created the agent_mcp_proxy_mappings table but did not
add indexes for the foreign-key columns; update the 023_create_mcp_proxies.go
migration to create indexes on agent_mcp_proxy_mappings(mcp_proxy_uuid) and
agent_mcp_proxy_mappings(artifact_uuid) (e.g., names like
idx_agent_mcp_proxy_mappings_mcp_proxy_uuid and
idx_agent_mcp_proxy_mappings_artifact_uuid) so FK cascade/lookup operations use
indexes; add the CREATE INDEX statements in the same migration after the CREATE
TABLE for the table and columns mcp_proxy_uuid and artifact_uuid.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@agent-manager-service/services/agent_mcp_proxy_service.go`:
- Around line 101-104: The mapping name is used directly in URL path segments
and must be escaped to avoid breaking URLs; update the code that trims req.Name
(variable name / mappingName) to validate non-empty then call url.PathEscape (or
equivalent path-segment encoder) before concatenating into the request path, and
replace raw uses of mappingName in the other occurrence (the block around the
293-305 region) with the escaped value so spaces, slashes and reserved chars are
encoded.

In `@agent-manager-service/services/gateway_events_service.go`:
- Around line 141-142: The BroadcastMCPProxyDeletionEvent currently emits
"mcpproxy.deleted" which breaks the existing undeploy event contract; update the
call in BroadcastMCPProxyDeletionEvent to use the existing undeploy event name
(e.g. "mcpproxy.undeployed") when invoking s.broadcastEvent (keep the HTTP
action "DELETE" and payload/event param the same) so MCP proxy removals align
with the backend's undeploy terminology used by other deployable artifacts.

In `@agent-manager-service/services/mcp_proxy_service.go`:
- Around line 412-419: Move the side-effectful operations (computing gatewayIDs
via gatewayIDsForDeletion, calling broadcastMCPProxyDeletion, and invoking
deleteAgentMappingsForOrgProxy) so they occur only after the source-of-truth
delete succeeds; specifically, call s.repo.Delete(handle, orgUUID) first and if
it returns nil then call s.deleteAgentMappingsForOrgProxy(ctx, orgUUID,
proxy.UUID) and s.broadcastMCPProxyDeletion(proxy, gatewayIDs) (compute
gatewayIDs after successful delete or compute beforehand but only broadcast
after success), returning the repo.Delete error immediately on failure so no
broadcasts or mapping deletions run when the DB delete fails.
- Around line 852-863: Mapper currently returns proxy.Configuration.Upstream
verbatim, exposing write-only credential values; instead sanitize the upstream
before setting MCPProxyDTO.Upstream by removing any auth.Value and emitting only
the stored secret reference (or nil) so credentials aren't leaked. Implement a
small sanitizer called from the mapper (or reuse/align with
prepareMCPUpstreamAuthForStorage semantics) that copies Upstream but replaces
Auth.Value with empty/nil and preserves SecretRef fields, then assign that
sanitized upstream to the MCPProxyDTO.Upstream field instead of
proxy.Configuration.Upstream.
- Around line 1235-1249: The parseMCPEventStream function currently returns only
the first "data:" line, which breaks multi-line SSE payloads; change it to
accumulate consecutive "data:" lines for a single event (joining them with '\n'
as per SSE), treating an empty line as the event boundary, and return the
assembled payload when the event completes (or at end if last event has no
trailing blank line); use the existing parseMCPEventStream symbol and replace
the single-line return with a buffer (e.g., bytes.Buffer or slice append) that
collects each trimmed data: value, joins them with newline, and returns the
joined bytes when an event finishes.
- Around line 1202-1215: The code uses io.ReadAll(resp.Body) unbounded; replace
it with a size-capped read using io.ReadAll(io.LimitReader(resp.Body,
maxMCPResponseBody+1)) (define a constant like maxMCPResponseBody = 10<<20 or an
appropriate limit) and if the resulting body length > maxMCPResponseBody return
an explicit error (e.g., ErrMCPResponseTooLarge) before any status or
event-stream parsing; apply this change around the existing resp, body handling
used with isMCPEventStream and parseMCPEventStream so oversized responses fail
fast and do not consume unbounded memory.

In
`@console/workspaces/libs/shared-component/src/utils/policyParameterEditor/schemaUtils.ts`:
- Around line 219-221: The hasMeaningfulValue function treats whitespace-only
strings as meaningful; update it to detect strings and return false for values
where typeof value === "string" and value.trim().length === 0, keeping the
existing undefined/null/empty string check and array recursion
(Array.isArray(value) ? value.some(hasMeaningfulValue) : ...), so that
whitespace-only strings are considered empty and won't cause branch-selection to
remain selected.

In `@console/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsx`:
- Around line 1115-1118: The formatRelativeTime function currently returns an
empty string for invalid timestamps, causing blank UI; update formatRelativeTime
(used for updatedAt rendering) to return a visible placeholder (e.g., "—" or
"Invalid date") when new Date(value).getTime() is NaN, and ensure the caller
rendering updatedAt displays that placeholder instead of blank text so users can
see an explicit fallback.

---

Duplicate comments:
In `@agent-manager-service/services/agent_mcp_proxy_service.go`:
- Around line 186-190: The code currently calls
s.mcpSvc.BroadcastMCPArtifactDeletion(*mapping.ArtifactUUID, orgName) before
removing the DB mapping via s.repo.Delete, which can trigger downstream removal
while the mapping still exists; move the BroadcastMCPArtifactDeletion call so it
only executes after s.repo.Delete returns no error (and mapping/ArtifactUUID are
non-nil), updating both places where this pattern appears (the block using
mapping.ArtifactUUID and the similar block later) to first perform delete, check
errors.Is(err, gorm.ErrRecordNotFound) / other errors, and only then call
s.mcpSvc.BroadcastMCPArtifactDeletion with agentName, projectName, orgName, name
and the dereferenced ArtifactUUID.

In
`@console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/PolicyParameterEditor.tsx`:
- Around line 261-263: When reinitializing form values in the useEffect that
calls setValues(initializeDefaultValues(parameters, existingValues)), also reset
the validation state to avoid stale errors; update the same useEffect (or the
initialize flow) to call setErrors(...) with an empty errors object or freshly
computed errors (e.g., via an initializeErrors helper) after setValues. Target
the useEffect that references initializeDefaultValues, the setValues state
setter, and the setErrors state setter to implement this change.

---

Nitpick comments:
In `@agent-manager-service/db_migrations/023_create_mcp_proxies.go`:
- Around line 38-56: The migration created the agent_mcp_proxy_mappings table
but did not add indexes for the foreign-key columns; update the
023_create_mcp_proxies.go migration to create indexes on
agent_mcp_proxy_mappings(mcp_proxy_uuid) and
agent_mcp_proxy_mappings(artifact_uuid) (e.g., names like
idx_agent_mcp_proxy_mappings_mcp_proxy_uuid and
idx_agent_mcp_proxy_mappings_artifact_uuid) so FK cascade/lookup operations use
indexes; add the CREATE INDEX statements in the same migration after the CREATE
TABLE for the table and columns mcp_proxy_uuid and artifact_uuid.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 84038d11-1041-4b0c-886f-7db294b1b532

📥 Commits

Reviewing files that changed from the base of the PR and between b523cbf and 7899ae6.

⛔ Files ignored due to path filters (1)
  • console/common/config/rush/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (84)
  • agent-manager-service/api/agent_mcp_proxy_routes.go
  • agent-manager-service/api/app.go
  • agent-manager-service/api/gateway_internal_routes.go
  • agent-manager-service/api/mcp_proxy_routes.go
  • agent-manager-service/controllers/agent_mcp_proxy_controller.go
  • agent-manager-service/controllers/gateway_internal_controller.go
  • agent-manager-service/controllers/mcp_proxy_controller.go
  • agent-manager-service/db_migrations/023_create_mcp_proxies.go
  • agent-manager-service/db_migrations/migration_list.go
  • agent-manager-service/models/agent_mcp_proxy_mapping.go
  • agent-manager-service/models/artifact.go
  • agent-manager-service/models/gateway_compat.go
  • agent-manager-service/models/mcp_proxy.go
  • agent-manager-service/repositories/agent_mcp_proxy_mapping_repository.go
  • agent-manager-service/repositories/artifact_repository.go
  • agent-manager-service/repositories/mcp_proxy_repository.go
  • agent-manager-service/services/agent_manager.go
  • agent-manager-service/services/agent_mcp_proxy_service.go
  • agent-manager-service/services/deployment_ack_handler.go
  • agent-manager-service/services/gateway_events_service.go
  • agent-manager-service/services/gateway_internal_service.go
  • agent-manager-service/services/mcp_proxy_deployment.go
  • agent-manager-service/services/mcp_proxy_service.go
  • agent-manager-service/services/platform_gateway_service.go
  • agent-manager-service/utils/api.go
  • agent-manager-service/utils/constants.go
  • agent-manager-service/utils/errors.go
  • agent-manager-service/wiring/params.go
  • agent-manager-service/wiring/wire.go
  • agent-manager-service/wiring/wire_gen.go
  • console/apps/web-ui/vite.config.ts
  • console/rush.json
  • console/workspaces/core-ui/package.json
  • console/workspaces/core-ui/src/Layouts/OxygenLayout/navigationItems.tsx
  • console/workspaces/core-ui/src/Route/Route.tsx
  • console/workspaces/core-ui/src/pages/index.tsx
  • console/workspaces/core-ui/vite.config.ts
  • console/workspaces/libs/api-client/src/apis/agent-mcp-proxies.ts
  • console/workspaces/libs/api-client/src/apis/index.ts
  • console/workspaces/libs/api-client/src/apis/mcp-proxies.ts
  • console/workspaces/libs/api-client/src/hooks/agent-mcp-proxies.ts
  • console/workspaces/libs/api-client/src/hooks/index.ts
  • console/workspaces/libs/api-client/src/hooks/mcp-policies.ts
  • console/workspaces/libs/api-client/src/hooks/mcp-proxies.ts
  • console/workspaces/libs/shared-component/package.json
  • console/workspaces/libs/shared-component/src/index.ts
  • console/workspaces/libs/shared-component/src/utils/policyParameterEditor/index.ts
  • console/workspaces/libs/shared-component/src/utils/policyParameterEditor/schemaUtils.ts
  • console/workspaces/libs/shared-component/src/utils/policyParameterEditor/types.ts
  • console/workspaces/libs/shared-component/src/utils/policyParameterEditor/yamlParser.ts
  • console/workspaces/libs/types/src/api/agent-mcp-proxies.ts
  • console/workspaces/libs/types/src/api/index.ts
  • console/workspaces/libs/types/src/api/mcp-proxies.ts
  • console/workspaces/libs/types/src/routes/generated-route.map.ts
  • console/workspaces/libs/types/src/routes/routes.map.ts
  • console/workspaces/pages/configure-agent/package.json
  • console/workspaces/pages/configure-agent/src/AddMCPServer.Component.tsx
  • console/workspaces/pages/configure-agent/src/Configure.Component.tsx
  • console/workspaces/pages/configure-agent/src/Configure/subComponents/AgentMCPServersSection.tsx
  • console/workspaces/pages/configure-agent/src/ViewMCPServer.Component.tsx
  • console/workspaces/pages/configure-agent/src/index.ts
  • console/workspaces/pages/llm-providers/src/PolicyParameterEditor/FieldRenderers.tsx
  • console/workspaces/pages/llm-providers/src/PolicyParameterEditor/PolicyParameterEditor.tsx
  • console/workspaces/pages/llm-providers/src/PolicyParameterEditor/SchemaTree.tsx
  • console/workspaces/pages/llm-providers/src/PolicyParameterEditor/yamlParser.ts
  • console/workspaces/pages/llm-providers/src/components/GuardrailSelectorDrawer/GuardrailSelectorDrawer.tsx
  • console/workspaces/pages/llm-providers/src/subComponents/GuardrailsSection.tsx
  • console/workspaces/pages/llm-providers/src/subComponents/LLMProviderGuardrailsTab.tsx
  • console/workspaces/pages/mcp-proxies/eslint.config.js
  • console/workspaces/pages/mcp-proxies/package.json
  • console/workspaces/pages/mcp-proxies/src/AddMCPProxy.Organization.tsx
  • console/workspaces/pages/mcp-proxies/src/MCPProxies.Organization.tsx
  • console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/FieldRenderers.tsx
  • console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/PolicyParameterEditor.tsx
  • console/workspaces/pages/mcp-proxies/src/PolicyParameterEditor/SchemaTree.tsx
  • console/workspaces/pages/mcp-proxies/src/components/MCPCapabilitiesView.tsx
  • console/workspaces/pages/mcp-proxies/src/components/MCPLogo.tsx
  • console/workspaces/pages/mcp-proxies/src/index.ts
  • console/workspaces/pages/mcp-proxies/src/subComponents/AddMCPProxyForm.tsx
  • console/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyTable.tsx
  • console/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsx
  • console/workspaces/pages/mcp-proxies/tsconfig.json
  • console/workspaces/pages/mcp-proxies/tsconfig.lib.json
  • console/workspaces/pages/mcp-proxies/vite.config.ts
💤 Files with no reviewable changes (1)
  • console/workspaces/pages/llm-providers/src/PolicyParameterEditor/yamlParser.ts
✅ Files skipped from review due to trivial changes (10)
  • console/workspaces/core-ui/package.json
  • console/workspaces/libs/types/src/api/index.ts
  • agent-manager-service/models/artifact.go
  • console/workspaces/pages/llm-providers/src/subComponents/LLMProviderGuardrailsTab.tsx
  • console/workspaces/libs/api-client/src/apis/index.ts
  • console/workspaces/libs/shared-component/package.json
  • console/workspaces/core-ui/vite.config.ts
  • console/workspaces/libs/types/src/api/mcp-proxies.ts
  • console/workspaces/pages/mcp-proxies/tsconfig.lib.json
  • agent-manager-service/api/gateway_internal_routes.go

Comment thread agent-manager-service/services/agent_mcp_proxy_service.go
Comment thread agent-manager-service/services/gateway_events_service.go Outdated
Comment thread agent-manager-service/services/mcp_proxy_service.go Outdated
Comment thread agent-manager-service/services/mcp_proxy_service.go
Comment thread agent-manager-service/services/mcp_proxy_service.go Outdated
Comment thread agent-manager-service/services/mcp_proxy_service.go
@Oshanath Oshanath force-pushed the feature-mcp-proxy branch from 7899ae6 to ca1f223 Compare May 26, 2026 11:06
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.

1 participant