UI implementation for Agent Identity#1302
Open
rasika2012 wants to merge 21 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds environment-scoped agent identity APIs, backend role/group retrieval, React Query hooks, identity management pages, MCP proxy identity security and endpoint editing, nested routing, shared UI components, and supporting type/configuration updates. ChangesAgent identity backend APIs
Agent identity console
MCP proxy console updates
Shared console UI and supporting updates
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
console/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsx (1)
79-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep TABS and TAB_SLUGS in a single array
TABSandTAB_SLUGSare index-aligned but maintained as separate arrays. Adding a tab to one without the other would silently break deep-linking. Consider a single array of{ label, slug }objects.♻️ Proposed refactor: single array of tab definitions
-const TABS = [ - "Overview", - "Capabilities", - "Connection", - "Manage Tools", - "Security", - "Rewrite", - "Policies", -] as const; - -const TAB_SLUGS = [ - "overview", - "capabilities", - "connection", - "manage-tools", - "security", - "rewrite", - "policies", -] as const; +const TAB_DEFS = [ + { label: "Overview", slug: "overview" }, + { label: "Capabilities", slug: "capabilities" }, + { label: "Connection", slug: "connection" }, + { label: "Manage Tools", slug: "manage-tools" }, + { label: "Security", slug: "security" }, + { label: "Rewrite", slug: "rewrite" }, + { label: "Policies", slug: "policies" }, +] as const; + +const TABS = TAB_DEFS.map((t) => t.label); +const TAB_SLUGS = TAB_DEFS.map((t) => t.slug);🤖 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 79 - 100, Consolidate the separate TABS and TAB_SLUGS arrays into one tab-definition array containing paired label and slug values, then update all consumers of TABS and TAB_SLUGS to read the corresponding label or slug properties. Preserve the existing tab order and URL slugs while ensuring selection and deep-linking use the unified definitions.console/workspaces/libs/api-client/src/apis/scopes.ts (1)
37-94: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
throw await res.json()can mask non-JSON error responses.If the server returns a non-JSON error body (e.g., plain text, HTML, or empty 500),
res.json()throws aSyntaxError, hiding the actual HTTP status and error. This pattern is used across all functions in this file and inagent-identity.ts. Consider wrapping with a fallback:♻️ Proposed refactor
- if (!res.ok) throw await res.json(); + if (!res.ok) { + let detail: unknown; + try { detail = await res.json(); } catch { detail = await res.text().catch(() => undefined); } + throw detail ?? new Error(`HTTP ${res.status}`); + }🤖 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/libs/api-client/src/apis/scopes.ts` around lines 37 - 94, Replace direct throw await res.json() error handling in listScopes, createScope, updateScope, and deleteScope with a shared fallback that attempts to parse JSON but preserves the HTTP status and response text when parsing fails, including empty or non-JSON bodies. Apply the same robust error-handling pattern to the related functions in agent-identity.ts.
🤖 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 `@console/workspaces/libs/api-client/src/apis/scopes.ts`:
- Around line 66-94: Guard against an undefined scopeName in both updateScope
and deleteScope before obtaining the token or making the HTTP request; reject
the operation using the existing project convention for invalid required path
parameters, and remove the scopeName ?? "" fallback from URL construction so
only a validated, encoded scope name is used.
In
`@console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/RoleEditPage.tsx`:
- Around line 107-139: Preserve role scopes that are absent from the catalog
when initializing selectedScopes. Update the initialization logic in the
selectedScopes useEffect to merge matching catalogScopes with placeholder
ScopeResponse entries such as { id: name, name } for every initialScopeNames
value not found in the catalog, so scopesDirty remains false initially and
updateRole retains them on save.
In
`@console/workspaces/pages/mcp-proxies/src/subComponents/EditMCPProxyDrawer.tsx`:
- Around line 85-96: Update the form-reset useEffect in EditMCPProxyDrawer to
run only when the drawer transitions from closed to open, rather than whenever
proxy changes while open. Track the previous open state with a ref or equivalent
transition guard, keep initializing from the current proxy on that transition,
and avoid including proxy as a trigger that can reset unsaved edits during
background refetches.
In
`@console/workspaces/pages/mcp-proxies/src/subComponents/MCPProxySecurityTab.tsx`:
- Around line 381-397: After a successful save in the update handler,
synchronize the React state with the normalized values that were persisted:
update identityMode to savedIdentityMode and toolScopeRows to the saved
tool-scope rows alongside lastSavedIdentityModeRef and
lastSavedToolScopeRowsRef. Ensure the isDirty/toolScopesDirty calculations use
these synchronized values so switching away from identity does not leave the
form dirty.
---
Nitpick comments:
In `@console/workspaces/libs/api-client/src/apis/scopes.ts`:
- Around line 37-94: Replace direct throw await res.json() error handling in
listScopes, createScope, updateScope, and deleteScope with a shared fallback
that attempts to parse JSON but preserves the HTTP status and response text when
parsing fails, including empty or non-JSON bodies. Apply the same robust
error-handling pattern to the related functions in agent-identity.ts.
In `@console/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsx`:
- Around line 79-100: Consolidate the separate TABS and TAB_SLUGS arrays into
one tab-definition array containing paired label and slug values, then update
all consumers of TABS and TAB_SLUGS to read the corresponding label or slug
properties. Preserve the existing tab order and URL slugs while ensuring
selection and deep-linking use the unified definitions.
🪄 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: 4d843555-d630-41c1-9935-60fcc84efe96
⛔ Files ignored due to path filters (1)
console/common/config/rush/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (45)
console/workspaces/libs/api-client/src/apis/agent-identity.tsconsole/workspaces/libs/api-client/src/apis/index.tsconsole/workspaces/libs/api-client/src/apis/scopes.tsconsole/workspaces/libs/api-client/src/hooks/agent-identity.tsconsole/workspaces/libs/api-client/src/hooks/index.tsconsole/workspaces/libs/api-client/src/hooks/react-query-notifications.tsconsole/workspaces/libs/api-client/src/hooks/scopes.tsconsole/workspaces/libs/shared-component/src/components/BackButton.tsxconsole/workspaces/libs/shared-component/src/components/EditFormSkeleton.tsxconsole/workspaces/libs/shared-component/src/components/EntityHeader.tsxconsole/workspaces/libs/shared-component/src/components/ListingSkeletonRows.tsxconsole/workspaces/libs/shared-component/src/components/index.tsconsole/workspaces/libs/types/src/api/agent-identity.tsconsole/workspaces/libs/types/src/api/index.tsconsole/workspaces/libs/types/src/api/llm-providers.tsconsole/workspaces/libs/types/src/api/mcp-proxies.tsconsole/workspaces/libs/types/src/api/scopes.tsconsole/workspaces/libs/types/src/routes/generated-route.map.tsconsole/workspaces/libs/types/src/routes/routes.map.tsconsole/workspaces/pages/env-thunders/package.jsonconsole/workspaces/pages/env-thunders/src/ThunderInstances.Organization.tsxconsole/workspaces/pages/env-thunders/src/subComponents/ThunderInstanceComingSoonTab.tsxconsole/workspaces/pages/env-thunders/src/subComponents/ViewThunderInstance.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/AgentsTab.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/GroupCreatePage.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/GroupEditPage.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/GroupsPage.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/RoleCreatePage.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/RoleEditPage.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/RolesPage.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/schemas.tsconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/useAgentLookup.tsconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/useAssignmentDelta.tsconsole/workspaces/pages/identities/src/GroupsPage.tsxconsole/workspaces/pages/identities/src/RolesPage.tsxconsole/workspaces/pages/identities/src/UsersPage.tsxconsole/workspaces/pages/mcp-proxies/package.jsonconsole/workspaces/pages/mcp-proxies/src/subComponents/EditMCPProxyDrawer.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyManageToolsTab.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyOverviewTab.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/MCPProxySecurityTab.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/ManageEndpointsDialog.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/mcpEndpoints.tsconsole/workspaces/pages/mcp-proxies/src/subComponents/useCopyWithFeedback.ts
💤 Files with no reviewable changes (1)
- console/workspaces/pages/env-thunders/src/subComponents/ThunderInstanceComingSoonTab.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@console/workspaces/pages/mcp-proxies/src/subComponents/EditMCPProxyDrawer.tsx`:
- Around line 130-139: Update EndpointsEditorSection to generate collision-proof
temporary IDs for newly added endpoint drafts instead of simple numeric values
such as "1". Ensure these IDs cannot match backend endpoint IDs, so the
existingByHandle lookup in the endpoint-mapping flow only reuses data for
genuinely existing endpoints and new drafts receive fresh identity and
configuration.
In
`@console/workspaces/pages/mcp-proxies/src/subComponents/EndpointFormFields.tsx`:
- Around line 123-152: The performFetch flow currently skips newer requests
while an older fetch is pending, allowing stale capabilities to overwrite the
latest URL. Update performFetch and its response-handling logic to let each
latest debounced fetch run, track request generations or equivalent identity,
and ignore results from older requests so only the newest URL’s response updates
fetchedInfo and related form state.
🪄 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: e1b2052d-6a22-449a-99ba-d867ee2a8aa2
⛔ Files ignored due to path filters (1)
console/common/config/rush/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
console/AGENTS.mdconsole/workspaces/pages/env-thunders/src/ThunderInstances.Organization.tsxconsole/workspaces/pages/env-thunders/src/subComponents/ThunderInstanceOverviewTab.tsxconsole/workspaces/pages/env-thunders/src/subComponents/ThunderInstancesTable.tsxconsole/workspaces/pages/env-thunders/src/subComponents/ViewThunderInstance.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/RoleEditPage.tsxconsole/workspaces/pages/llm-providers/src/subComponents/LLMProviderSecurityTab.tsxconsole/workspaces/pages/mcp-proxies/package.jsonconsole/workspaces/pages/mcp-proxies/src/components/MCPCapabilitiesView.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/AddEndpointDialog.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/AddMCPProxyForm.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/EditMCPProxyDrawer.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/EndpointFormFields.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/EndpointRow.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/EndpointsEditorSection.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyManageToolsTab.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyOverviewTab.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyRewriteTab.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/MCPProxySecurityTab.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/ManageEndpointsDialog.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/mcpEndpoints.ts
💤 Files with no reviewable changes (2)
- console/workspaces/pages/mcp-proxies/src/subComponents/ManageEndpointsDialog.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/AddEndpointDialog.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- console/workspaces/pages/env-thunders/src/ThunderInstances.Organization.tsx
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/RoleEditPage.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/mcpEndpoints.ts
- console/workspaces/pages/env-thunders/src/subComponents/ViewThunderInstance.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyOverviewTab.tsx
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
console/workspaces/pages/mcp-proxies/src/subComponents/EndpointsEditorSection.tsx (1)
235-239: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCount only currently available environments.
usedEnvIds.sizeincludes IDs present in endpoint drafts even when those environments are no longer inenvironments. This can display invalid values such as “2 of 1 environments have an endpoint.” Count the intersection with the current environment list instead.Proposed fix
+ const configuredEnvironmentCount = environments.filter( + (environment) => environment.id && usedEnvIds.has(environment.id), + ).length; + {environments.length > 0 ? ( <Typography variant="caption" color="text.secondary"> - {usedEnvIds.size} of {environments.length} environments have an + {configuredEnvironmentCount} of {environments.length} environments have an endpoint.🤖 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/EndpointsEditorSection.tsx` around lines 235 - 239, Update the environment count display in EndpointsEditorSection to count only IDs from usedEnvIds that match currently available environments, using the environments collection as the source of valid IDs. Keep environments.length as the denominator and preserve the existing text and conditional rendering.
🤖 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.
Outside diff comments:
In
`@console/workspaces/pages/mcp-proxies/src/subComponents/EndpointsEditorSection.tsx`:
- Around line 235-239: Update the environment count display in
EndpointsEditorSection to count only IDs from usedEnvIds that match currently
available environments, using the environments collection as the source of valid
IDs. Keep environments.length as the denominator and preserve the existing text
and conditional rendering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 34c23a81-8dde-40ae-bea6-43674eeeef57
📒 Files selected for processing (7)
console/workspaces/libs/api-client/src/apis/scopes.tsconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/RoleEditPage.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/EditMCPProxyDrawer.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/EndpointFormFields.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/EndpointsEditorSection.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/MCPProxySecurityTab.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- console/workspaces/libs/api-client/src/apis/scopes.ts
- console/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/EditMCPProxyDrawer.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/EndpointFormFields.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/MCPProxySecurityTab.tsx
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/RoleEditPage.tsx
… and ListingSkeletonRows
…ut and remove unused key properties
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
agent-manager-service/docs/api_v1_openapi.yaml (2)
6297-6420: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInconsistent tag naming for new endpoints.
New paths use tags
[MCPProxyScopes](6302, 6326, 6362, 6398) and[AgentIdentities](10578), diverging from this file's established Title-Case space-separated convention (e.g. "MCP Proxies", "MCP API Keys", "Agent Identity"). This will fragment the generated Swagger/docs grouping.✏️ Proposed fix
- tags: [MCPProxyScopes] + tags: [MCP Proxies]- tags: [AgentIdentities] + tags: [Agent Identity]Also applies to: 10573-10599
🤖 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/docs/api_v1_openapi.yaml` around lines 6297 - 6420, Update the tags on the new MCP proxy scope operations to use the established display name “MCP Proxies” rather than “MCPProxyScopes”, and update the Agent Identities endpoint near the Agent identity operations to use the existing “Agent Identity” convention instead of “AgentIdentities”. Keep each endpoint’s operationId and behavior unchanged.
6297-6420: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winNew MCP Proxy Scopes endpoints omit
ErrorResponseschema for error responses.Unlike virtually every other endpoint in this file, the 400/404/409 responses for
listMCPProxyScopes,createMCPProxyScope,updateMCPProxyScope, anddeleteMCPProxyScopeonly have adescription, with nocontent/schemareferencing#/components/schemas/ErrorResponse. This means generated clients won't decode structured errors for these status codes, unlike sibling operations.🤖 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/docs/api_v1_openapi.yaml` around lines 6297 - 6420, Update the error responses for listMCPProxyScopes, createMCPProxyScope, updateMCPProxyScope, and deleteMCPProxyScope to include application/json content whose schema references `#/components/schemas/ErrorResponse`, matching sibling endpoints. Preserve each existing status code and description while adding the structured error schema to every declared 400, 404, and 409 response.agent-manager-service/services/agent_thunder_provisioning_service.go (1)
511-566: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared binding-lookup helper to remove duplication.
GetAgentRoles(511-523) andGetAgentGroups(537-549) duplicate the same binding lookup +ThunderAgentIDempty-check block verbatim.♻️ Proposed refactor
+func (s *agentThunderProvisioningService) resolveProvisionedBinding(ctx context.Context, ouID, projectName, agentName, envName string) (*models.AgentThunderClient, error) { + binding, err := s.repo.Get(ctx, ouID, projectName, agentName, envName) + if err != nil { + if errors.Is(err, repositories.ErrAgentThunderClientNotFound) { + return nil, fmt.Errorf("%w: %s in %s", utils.ErrAgentIdentityNotProvisioned, agentName, envName) + } + return nil, err + } + if binding.ThunderAgentID == "" { + return nil, fmt.Errorf("%w: %s in %s", utils.ErrAgentIdentityNotProvisioned, agentName, envName) + } + return binding, nil +} + func (s *agentThunderProvisioningService) GetAgentRoles(ctx context.Context, ouID, projectName, agentName, envName string) ([]thundersvc.ThunderRole, error) { - binding, err := s.repo.Get(ctx, ouID, projectName, agentName, envName) - if err != nil { - if errors.Is(err, repositories.ErrAgentThunderClientNotFound) { - return nil, fmt.Errorf("%w: %s in %s", utils.ErrAgentIdentityNotProvisioned, agentName, envName) - } - return nil, err - } - if binding.ThunderAgentID == "" { - return nil, fmt.Errorf("%w: %s in %s", utils.ErrAgentIdentityNotProvisioned, agentName, envName) - } + binding, err := s.resolveProvisionedBinding(ctx, ouID, projectName, agentName, envName) + if err != nil { + return nil, err + }Apply the analogous change to
GetAgentGroups.🤖 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_thunder_provisioning_service.go` around lines 511 - 566, Extract the duplicated repository lookup and empty ThunderAgentID validation from GetAgentRoles and GetAgentGroups into a shared helper on agentThunderProvisioningService. Have the helper return the binding or the existing ErrAgentIdentityNotProvisioned-wrapped error, then update both methods to reuse it while preserving their current downstream role and group retrieval behavior.
🤖 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`:
- Line 2192: Update both call sites of cleanupAgentMonitors in DeleteAgent to
dispatch the cleanup asynchronously with go, matching the existing non-blocking
post-delete cleanup pattern. Preserve the current arguments and ensure both
paths return without waiting for cleanupAgentMonitors or DeleteMonitorsByAgent
to complete.
- Around line 3055-3062: Update the error logging in visiblePipelineEnvironments
to include ouID alongside projectName and error, providing the organization
identifier as correlation context when GetProjectDeploymentPipeline fails.
---
Nitpick comments:
In `@agent-manager-service/docs/api_v1_openapi.yaml`:
- Around line 6297-6420: Update the tags on the new MCP proxy scope operations
to use the established display name “MCP Proxies” rather than “MCPProxyScopes”,
and update the Agent Identities endpoint near the Agent identity operations to
use the existing “Agent Identity” convention instead of “AgentIdentities”. Keep
each endpoint’s operationId and behavior unchanged.
- Around line 6297-6420: Update the error responses for listMCPProxyScopes,
createMCPProxyScope, updateMCPProxyScope, and deleteMCPProxyScope to include
application/json content whose schema references
`#/components/schemas/ErrorResponse`, matching sibling endpoints. Preserve each
existing status code and description while adding the structured error schema to
every declared 400, 404, and 409 response.
In `@agent-manager-service/services/agent_thunder_provisioning_service.go`:
- Around line 511-566: Extract the duplicated repository lookup and empty
ThunderAgentID validation from GetAgentRoles and GetAgentGroups into a shared
helper on agentThunderProvisioningService. Have the helper return the binding or
the existing ErrAgentIdentityNotProvisioned-wrapped error, then update both
methods to reuse it while preserving their current downstream role and group
retrieval behavior.
🪄 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: afe19c95-0fa8-405a-b6e3-7fff227b9822
⛔ Files ignored due to path filters (1)
console/common/config/rush/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (67)
agent-manager-service/api/agent_routes.goagent-manager-service/clients/clientmocks/env_identity_client_mock.goagent-manager-service/clients/thundersvc/identity_client.goagent-manager-service/controllers/agent_controller.goagent-manager-service/docs/api_v1_openapi.yamlagent-manager-service/services/agent_manager.goagent-manager-service/services/agent_manager_test.goagent-manager-service/services/agent_thunder_provisioning_service.goagent-manager-service/services/agent_thunder_provisioning_service_test.goagent-manager-service/spec/api_agent_identity.goconsole/AGENTS.mdconsole/workspaces/libs/api-client/src/apis/agent-identity.tsconsole/workspaces/libs/api-client/src/apis/index.tsconsole/workspaces/libs/api-client/src/apis/mcp-proxy-scopes.tsconsole/workspaces/libs/api-client/src/hooks/agent-identity.tsconsole/workspaces/libs/api-client/src/hooks/index.tsconsole/workspaces/libs/api-client/src/hooks/mcp-proxy-scopes.tsconsole/workspaces/libs/api-client/src/hooks/react-query-notifications.tsconsole/workspaces/libs/shared-component/src/components/BackButton.tsxconsole/workspaces/libs/shared-component/src/components/EditFormSkeleton.tsxconsole/workspaces/libs/shared-component/src/components/EntityHeader.tsxconsole/workspaces/libs/shared-component/src/components/ListingSkeletonRows.tsxconsole/workspaces/libs/shared-component/src/components/index.tsconsole/workspaces/libs/types/src/api/agent-identity.tsconsole/workspaces/libs/types/src/api/index.tsconsole/workspaces/libs/types/src/api/llm-providers.tsconsole/workspaces/libs/types/src/api/mcp-proxies.tsconsole/workspaces/libs/types/src/api/mcp-proxy-scopes.tsconsole/workspaces/libs/types/src/routes/generated-route.map.tsconsole/workspaces/libs/types/src/routes/routes.map.tsconsole/workspaces/pages/env-thunders/package.jsonconsole/workspaces/pages/env-thunders/src/ThunderInstances.Organization.tsxconsole/workspaces/pages/env-thunders/src/subComponents/ThunderInstanceComingSoonTab.tsxconsole/workspaces/pages/env-thunders/src/subComponents/ThunderInstanceOverviewTab.tsxconsole/workspaces/pages/env-thunders/src/subComponents/ThunderInstancesTable.tsxconsole/workspaces/pages/env-thunders/src/subComponents/ViewThunderInstance.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/AgentsTab.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/GroupCreatePage.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/GroupEditPage.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/GroupsPage.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/RoleCreatePage.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/RoleEditPage.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/RolesPage.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/schemas.tsconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/scopeChoice.tsconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/useAgentLookup.tsconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/useAssignmentDelta.tsconsole/workspaces/pages/identities/src/GroupsPage.tsxconsole/workspaces/pages/identities/src/RolesPage.tsxconsole/workspaces/pages/identities/src/UsersPage.tsxconsole/workspaces/pages/llm-providers/src/subComponents/LLMProviderSecurityTab.tsxconsole/workspaces/pages/mcp-proxies/package.jsonconsole/workspaces/pages/mcp-proxies/src/components/MCPCapabilitiesView.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/AddEndpointDialog.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/AddMCPProxyForm.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/EditMCPProxyDrawer.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/EndpointFormFields.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/EndpointRow.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/EndpointsEditorSection.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyManageToolsTab.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyOverviewTab.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyRewriteTab.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/MCPProxySecurityTab.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/ManageEndpointsDialog.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/mcpEndpoints.tsconsole/workspaces/pages/mcp-proxies/src/subComponents/useCopyWithFeedback.ts
💤 Files with no reviewable changes (3)
- console/workspaces/pages/env-thunders/src/subComponents/ThunderInstanceComingSoonTab.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/ManageEndpointsDialog.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/AddEndpointDialog.tsx
🚧 Files skipped from review as they are similar to previous changes (43)
- console/AGENTS.md
- console/workspaces/pages/identities/src/UsersPage.tsx
- console/workspaces/libs/api-client/src/hooks/index.ts
- console/workspaces/libs/api-client/src/apis/index.ts
- console/workspaces/libs/shared-component/src/components/index.ts
- console/workspaces/libs/shared-component/src/components/BackButton.tsx
- console/workspaces/libs/types/src/api/index.ts
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/schemas.ts
- console/workspaces/pages/env-thunders/package.json
- console/workspaces/pages/mcp-proxies/src/subComponents/EndpointRow.tsx
- console/workspaces/libs/types/src/routes/routes.map.ts
- console/workspaces/libs/shared-component/src/components/EditFormSkeleton.tsx
- console/workspaces/pages/env-thunders/src/ThunderInstances.Organization.tsx
- console/workspaces/libs/api-client/src/hooks/react-query-notifications.ts
- console/workspaces/libs/shared-component/src/components/EntityHeader.tsx
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/useAssignmentDelta.ts
- console/workspaces/libs/shared-component/src/components/ListingSkeletonRows.tsx
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/useAgentLookup.ts
- console/workspaces/libs/types/src/routes/generated-route.map.ts
- console/workspaces/pages/identities/src/GroupsPage.tsx
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/GroupEditPage.tsx
- console/workspaces/libs/api-client/src/apis/agent-identity.ts
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/AgentsTab.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/EndpointFormFields.tsx
- console/workspaces/pages/env-thunders/src/subComponents/ThunderInstancesTable.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyOverviewTab.tsx
- console/workspaces/pages/mcp-proxies/src/components/MCPCapabilitiesView.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/AddMCPProxyForm.tsx
- console/workspaces/libs/api-client/src/hooks/agent-identity.ts
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/RoleEditPage.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/mcpEndpoints.ts
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/RoleCreatePage.tsx
- console/workspaces/pages/env-thunders/src/subComponents/ThunderInstanceOverviewTab.tsx
- console/workspaces/pages/mcp-proxies/package.json
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/GroupsPage.tsx
- console/workspaces/pages/env-thunders/src/subComponents/ViewThunderInstance.tsx
- console/workspaces/libs/types/src/api/agent-identity.ts
- console/workspaces/pages/mcp-proxies/src/subComponents/EndpointsEditorSection.tsx
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/RolesPage.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyManageToolsTab.tsx
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/GroupCreatePage.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/EditMCPProxyDrawer.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyRewriteTab.tsx
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🧹 Nitpick comments (3)
agent-manager-service/docs/api_v1_openapi.yaml (2)
6297-6420: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInconsistent tag naming for new endpoints.
New paths use tags
[MCPProxyScopes](6302, 6326, 6362, 6398) and[AgentIdentities](10578), diverging from this file's established Title-Case space-separated convention (e.g. "MCP Proxies", "MCP API Keys", "Agent Identity"). This will fragment the generated Swagger/docs grouping.✏️ Proposed fix
- tags: [MCPProxyScopes] + tags: [MCP Proxies]- tags: [AgentIdentities] + tags: [Agent Identity]Also applies to: 10573-10599
🤖 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/docs/api_v1_openapi.yaml` around lines 6297 - 6420, Update the tags on the new MCP proxy scope operations to use the established display name “MCP Proxies” rather than “MCPProxyScopes”, and update the Agent Identities endpoint near the Agent identity operations to use the existing “Agent Identity” convention instead of “AgentIdentities”. Keep each endpoint’s operationId and behavior unchanged.
6297-6420: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winNew MCP Proxy Scopes endpoints omit
ErrorResponseschema for error responses.Unlike virtually every other endpoint in this file, the 400/404/409 responses for
listMCPProxyScopes,createMCPProxyScope,updateMCPProxyScope, anddeleteMCPProxyScopeonly have adescription, with nocontent/schemareferencing#/components/schemas/ErrorResponse. This means generated clients won't decode structured errors for these status codes, unlike sibling operations.🤖 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/docs/api_v1_openapi.yaml` around lines 6297 - 6420, Update the error responses for listMCPProxyScopes, createMCPProxyScope, updateMCPProxyScope, and deleteMCPProxyScope to include application/json content whose schema references `#/components/schemas/ErrorResponse`, matching sibling endpoints. Preserve each existing status code and description while adding the structured error schema to every declared 400, 404, and 409 response.agent-manager-service/services/agent_thunder_provisioning_service.go (1)
511-566: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared binding-lookup helper to remove duplication.
GetAgentRoles(511-523) andGetAgentGroups(537-549) duplicate the same binding lookup +ThunderAgentIDempty-check block verbatim.♻️ Proposed refactor
+func (s *agentThunderProvisioningService) resolveProvisionedBinding(ctx context.Context, ouID, projectName, agentName, envName string) (*models.AgentThunderClient, error) { + binding, err := s.repo.Get(ctx, ouID, projectName, agentName, envName) + if err != nil { + if errors.Is(err, repositories.ErrAgentThunderClientNotFound) { + return nil, fmt.Errorf("%w: %s in %s", utils.ErrAgentIdentityNotProvisioned, agentName, envName) + } + return nil, err + } + if binding.ThunderAgentID == "" { + return nil, fmt.Errorf("%w: %s in %s", utils.ErrAgentIdentityNotProvisioned, agentName, envName) + } + return binding, nil +} + func (s *agentThunderProvisioningService) GetAgentRoles(ctx context.Context, ouID, projectName, agentName, envName string) ([]thundersvc.ThunderRole, error) { - binding, err := s.repo.Get(ctx, ouID, projectName, agentName, envName) - if err != nil { - if errors.Is(err, repositories.ErrAgentThunderClientNotFound) { - return nil, fmt.Errorf("%w: %s in %s", utils.ErrAgentIdentityNotProvisioned, agentName, envName) - } - return nil, err - } - if binding.ThunderAgentID == "" { - return nil, fmt.Errorf("%w: %s in %s", utils.ErrAgentIdentityNotProvisioned, agentName, envName) - } + binding, err := s.resolveProvisionedBinding(ctx, ouID, projectName, agentName, envName) + if err != nil { + return nil, err + }Apply the analogous change to
GetAgentGroups.🤖 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_thunder_provisioning_service.go` around lines 511 - 566, Extract the duplicated repository lookup and empty ThunderAgentID validation from GetAgentRoles and GetAgentGroups into a shared helper on agentThunderProvisioningService. Have the helper return the binding or the existing ErrAgentIdentityNotProvisioned-wrapped error, then update both methods to reuse it while preserving their current downstream role and group retrieval behavior.
🤖 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`:
- Line 2192: Update both call sites of cleanupAgentMonitors in DeleteAgent to
dispatch the cleanup asynchronously with go, matching the existing non-blocking
post-delete cleanup pattern. Preserve the current arguments and ensure both
paths return without waiting for cleanupAgentMonitors or DeleteMonitorsByAgent
to complete.
- Around line 3055-3062: Update the error logging in visiblePipelineEnvironments
to include ouID alongside projectName and error, providing the organization
identifier as correlation context when GetProjectDeploymentPipeline fails.
---
Nitpick comments:
In `@agent-manager-service/docs/api_v1_openapi.yaml`:
- Around line 6297-6420: Update the tags on the new MCP proxy scope operations
to use the established display name “MCP Proxies” rather than “MCPProxyScopes”,
and update the Agent Identities endpoint near the Agent identity operations to
use the existing “Agent Identity” convention instead of “AgentIdentities”. Keep
each endpoint’s operationId and behavior unchanged.
- Around line 6297-6420: Update the error responses for listMCPProxyScopes,
createMCPProxyScope, updateMCPProxyScope, and deleteMCPProxyScope to include
application/json content whose schema references
`#/components/schemas/ErrorResponse`, matching sibling endpoints. Preserve each
existing status code and description while adding the structured error schema to
every declared 400, 404, and 409 response.
In `@agent-manager-service/services/agent_thunder_provisioning_service.go`:
- Around line 511-566: Extract the duplicated repository lookup and empty
ThunderAgentID validation from GetAgentRoles and GetAgentGroups into a shared
helper on agentThunderProvisioningService. Have the helper return the binding or
the existing ErrAgentIdentityNotProvisioned-wrapped error, then update both
methods to reuse it while preserving their current downstream role and group
retrieval behavior.
🪄 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: afe19c95-0fa8-405a-b6e3-7fff227b9822
⛔ Files ignored due to path filters (1)
console/common/config/rush/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (67)
agent-manager-service/api/agent_routes.goagent-manager-service/clients/clientmocks/env_identity_client_mock.goagent-manager-service/clients/thundersvc/identity_client.goagent-manager-service/controllers/agent_controller.goagent-manager-service/docs/api_v1_openapi.yamlagent-manager-service/services/agent_manager.goagent-manager-service/services/agent_manager_test.goagent-manager-service/services/agent_thunder_provisioning_service.goagent-manager-service/services/agent_thunder_provisioning_service_test.goagent-manager-service/spec/api_agent_identity.goconsole/AGENTS.mdconsole/workspaces/libs/api-client/src/apis/agent-identity.tsconsole/workspaces/libs/api-client/src/apis/index.tsconsole/workspaces/libs/api-client/src/apis/mcp-proxy-scopes.tsconsole/workspaces/libs/api-client/src/hooks/agent-identity.tsconsole/workspaces/libs/api-client/src/hooks/index.tsconsole/workspaces/libs/api-client/src/hooks/mcp-proxy-scopes.tsconsole/workspaces/libs/api-client/src/hooks/react-query-notifications.tsconsole/workspaces/libs/shared-component/src/components/BackButton.tsxconsole/workspaces/libs/shared-component/src/components/EditFormSkeleton.tsxconsole/workspaces/libs/shared-component/src/components/EntityHeader.tsxconsole/workspaces/libs/shared-component/src/components/ListingSkeletonRows.tsxconsole/workspaces/libs/shared-component/src/components/index.tsconsole/workspaces/libs/types/src/api/agent-identity.tsconsole/workspaces/libs/types/src/api/index.tsconsole/workspaces/libs/types/src/api/llm-providers.tsconsole/workspaces/libs/types/src/api/mcp-proxies.tsconsole/workspaces/libs/types/src/api/mcp-proxy-scopes.tsconsole/workspaces/libs/types/src/routes/generated-route.map.tsconsole/workspaces/libs/types/src/routes/routes.map.tsconsole/workspaces/pages/env-thunders/package.jsonconsole/workspaces/pages/env-thunders/src/ThunderInstances.Organization.tsxconsole/workspaces/pages/env-thunders/src/subComponents/ThunderInstanceComingSoonTab.tsxconsole/workspaces/pages/env-thunders/src/subComponents/ThunderInstanceOverviewTab.tsxconsole/workspaces/pages/env-thunders/src/subComponents/ThunderInstancesTable.tsxconsole/workspaces/pages/env-thunders/src/subComponents/ViewThunderInstance.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/AgentsTab.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/GroupCreatePage.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/GroupEditPage.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/GroupsPage.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/RoleCreatePage.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/RoleEditPage.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/RolesPage.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/schemas.tsconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/scopeChoice.tsconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/useAgentLookup.tsconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/useAssignmentDelta.tsconsole/workspaces/pages/identities/src/GroupsPage.tsxconsole/workspaces/pages/identities/src/RolesPage.tsxconsole/workspaces/pages/identities/src/UsersPage.tsxconsole/workspaces/pages/llm-providers/src/subComponents/LLMProviderSecurityTab.tsxconsole/workspaces/pages/mcp-proxies/package.jsonconsole/workspaces/pages/mcp-proxies/src/components/MCPCapabilitiesView.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/AddEndpointDialog.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/AddMCPProxyForm.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/EditMCPProxyDrawer.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/EndpointFormFields.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/EndpointRow.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/EndpointsEditorSection.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyManageToolsTab.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyOverviewTab.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyRewriteTab.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/MCPProxySecurityTab.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/ManageEndpointsDialog.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/ViewMCPProxy.tsxconsole/workspaces/pages/mcp-proxies/src/subComponents/mcpEndpoints.tsconsole/workspaces/pages/mcp-proxies/src/subComponents/useCopyWithFeedback.ts
💤 Files with no reviewable changes (3)
- console/workspaces/pages/env-thunders/src/subComponents/ThunderInstanceComingSoonTab.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/ManageEndpointsDialog.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/AddEndpointDialog.tsx
🚧 Files skipped from review as they are similar to previous changes (43)
- console/AGENTS.md
- console/workspaces/pages/identities/src/UsersPage.tsx
- console/workspaces/libs/api-client/src/hooks/index.ts
- console/workspaces/libs/api-client/src/apis/index.ts
- console/workspaces/libs/shared-component/src/components/index.ts
- console/workspaces/libs/shared-component/src/components/BackButton.tsx
- console/workspaces/libs/types/src/api/index.ts
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/schemas.ts
- console/workspaces/pages/env-thunders/package.json
- console/workspaces/pages/mcp-proxies/src/subComponents/EndpointRow.tsx
- console/workspaces/libs/types/src/routes/routes.map.ts
- console/workspaces/libs/shared-component/src/components/EditFormSkeleton.tsx
- console/workspaces/pages/env-thunders/src/ThunderInstances.Organization.tsx
- console/workspaces/libs/api-client/src/hooks/react-query-notifications.ts
- console/workspaces/libs/shared-component/src/components/EntityHeader.tsx
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/useAssignmentDelta.ts
- console/workspaces/libs/shared-component/src/components/ListingSkeletonRows.tsx
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/useAgentLookup.ts
- console/workspaces/libs/types/src/routes/generated-route.map.ts
- console/workspaces/pages/identities/src/GroupsPage.tsx
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/GroupEditPage.tsx
- console/workspaces/libs/api-client/src/apis/agent-identity.ts
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/AgentsTab.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/EndpointFormFields.tsx
- console/workspaces/pages/env-thunders/src/subComponents/ThunderInstancesTable.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyOverviewTab.tsx
- console/workspaces/pages/mcp-proxies/src/components/MCPCapabilitiesView.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/AddMCPProxyForm.tsx
- console/workspaces/libs/api-client/src/hooks/agent-identity.ts
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/RoleEditPage.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/mcpEndpoints.ts
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/RoleCreatePage.tsx
- console/workspaces/pages/env-thunders/src/subComponents/ThunderInstanceOverviewTab.tsx
- console/workspaces/pages/mcp-proxies/package.json
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/GroupsPage.tsx
- console/workspaces/pages/env-thunders/src/subComponents/ViewThunderInstance.tsx
- console/workspaces/libs/types/src/api/agent-identity.ts
- console/workspaces/pages/mcp-proxies/src/subComponents/EndpointsEditorSection.tsx
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/RolesPage.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyManageToolsTab.tsx
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/GroupCreatePage.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/EditMCPProxyDrawer.tsx
- console/workspaces/pages/mcp-proxies/src/subComponents/MCPProxyRewriteTab.tsx
🛑 Comments failed to post (2)
agent-manager-service/services/agent_manager.go (2)
2192-2192: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
cleanupAgentMonitorsis called synchronously, contradicting its own doc comment and the sibling "never blocks the caller" pattern.Both call sites (2192, 2218) invoke
cleanupAgentMonitorswithoutgo, while the function's doc comment claims it "match[es] the other post-delete cleanups" — which are all dispatched as background goroutines (go s.deleteAgentLLMConfigurations(...),go s.agentThunderProvisioning.DeleteAllBindings(...)). As written,DeleteAgent's HTTP response is now blocked onDeleteMonitorsByAgentcompleting, adding avoidable latency for a documented best-effort operation.🐛 Proposed fix
- s.cleanupAgentMonitors(ctx, ouID, projectName, agentName) + go s.cleanupAgentMonitors(ctx, ouID, projectName, agentName) return niland
- // Cleanup monitors owned by this agent so they are not orphaned after deletion. - s.cleanupAgentMonitors(ctx, ouID, projectName, agentName) + // Cleanup monitors owned by this agent so they are not orphaned after deletion. + go s.cleanupAgentMonitors(ctx, ouID, projectName, agentName)Also applies to: 2217-2230
🤖 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_manager.go` at line 2192, Update both call sites of cleanupAgentMonitors in DeleteAgent to dispatch the cleanup asynchronously with go, matching the existing non-blocking post-delete cleanup pattern. Preserve the current arguments and ensure both paths return without waiting for cleanupAgentMonitors or DeleteMonitorsByAgent to complete.
3055-3062: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add
ouIDto the error log for correlation.The log only includes
projectName, omitting the organization identifier.Based on path instructions: "Log with correlation context including organization, resource ID, and request ID."📝 Proposed fix
- s.logger.Error("Failed to get deployment pipeline for agent identity visibility", "projectName", projectName, "error", err) + s.logger.Error("Failed to get deployment pipeline for agent identity visibility", "ouID", ouID, "projectName", projectName, "error", err)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func (s *agentManagerService) visiblePipelineEnvironments(ctx context.Context, ouID, projectName string) (map[string]bool, error) { pipeline, err := s.ocClient.GetProjectDeploymentPipeline(ctx, ouID, projectName) if err != nil { s.logger.Error("Failed to get deployment pipeline for agent identity visibility", "ouID", ouID, "projectName", projectName, "error", err) return nil, translatePipelineError(err) } return allPipelineEnvironmentNames(pipeline.PromotionPaths), nil }🤖 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_manager.go` around lines 3055 - 3062, Update the error logging in visiblePipelineEnvironments to include ouID alongside projectName and error, providing the organization identifier as correlation context when GetProjectDeploymentPipeline fails.Source: Path instructions
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
console/workspaces/libs/api-client/src/apis/agents.ts (1)
206-236: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unreachable
if (!res.ok)branches
httpGETalready throws on non-OK responses, and the same dead check is repeated across the GET helpers inagents.ts. Folding these calls through a shared helper would remove the duplication as well.🤖 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/libs/api-client/src/apis/agents.ts` around lines 206 - 236, Remove the unreachable !res.ok checks from getAgentRoles and getAgentGroups, relying on httpGET to throw for non-OK responses before returning res.json(). Apply the same cleanup across the repeated GET helpers in agents.ts, and reuse a shared helper for the common request/error/JSON flow where appropriate.
🤖 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.
Nitpick comments:
In `@console/workspaces/libs/api-client/src/apis/agents.ts`:
- Around line 206-236: Remove the unreachable !res.ok checks from getAgentRoles
and getAgentGroups, relying on httpGET to throw for non-OK responses before
returning res.json(). Apply the same cleanup across the repeated GET helpers in
agents.ts, and reuse a shared helper for the common request/error/JSON flow
where appropriate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 55c10e00-397a-4257-8d67-13fc77e8031d
⛔ Files ignored due to path filters (1)
console/common/config/rush/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
console/workspaces/libs/api-client/src/apis/agents.tsconsole/workspaces/libs/api-client/src/hooks/agents.tsconsole/workspaces/libs/types/src/api/agents.tsconsole/workspaces/libs/types/src/routes/generated-route.map.tsconsole/workspaces/libs/types/src/routes/routes.map.tsconsole/workspaces/pages/env-thunders/src/subComponents/ViewThunderInstance.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/AgentDetailPage.tsxconsole/workspaces/pages/env-thunders/src/subComponents/agentIdentity/AgentsTab.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- console/workspaces/libs/types/src/routes/routes.map.ts
- console/workspaces/libs/types/src/routes/generated-route.map.ts
- console/workspaces/pages/env-thunders/src/subComponents/agentIdentity/AgentsTab.tsx
- console/workspaces/pages/env-thunders/src/subComponents/ViewThunderInstance.tsx
…provisioning, regeneration, and revocation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Issue: #1269
Goals
Approach
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit