Fix UI bugs in AI Workspace - #2909
Conversation
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (24)
📝 WalkthroughWalkthroughThe changes add typed duplicate API-key conflicts, external endpoint and context validation, paginated provider-template retrieval, relative URL resolution, shared cost-policy handling, dynamic template logos, and API-derived API-key error messages. ChangesProvider API and workspace fixes
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@gateway/gateway-runtime/policy-engine/internal/analytics/analytics.go`:
- Around line 149-160: Update Process so internal loopback provider events are
suppressed only for analytics publishers, while the traffic-log publisher still
receives them. Preserve the existing suppression behavior for Moesif or other
analytics publishers, using the publisher classification established by
NewAnalytics. Add a regression test covering loopback LlmProvider traffic with
analytics and traffic logging enabled.
In `@platform-api/internal/repository/apikey.go`:
- Around line 58-63: Update the duplicate-error handling in the API key creation
flow to return APIKeyExists only when the violated uniqueness constraint is
specifically (artifact_uuid, handle). Use constraint-specific detection or
verify the conflicting key before mapping the error; leave other duplicate-key
failures on the existing generic error path.
In `@platform-api/internal/service/llm.go`:
- Around line 359-365: Strengthen validation in the metadata endpointUrl
handling around utils.ValidateURL so user-supplied URLs reject userinfo, unsafe
schemes, and private, link-local, metadata, or otherwise unsafe resolved IP
destinations before storage. Apply the same protection consistently to both
create and update flows, or ensure every consumer performs equivalent
resolved-IP and dial-time checks before making network requests.
🪄 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 Plus
Run ID: 62b48acc-bc33-4a17-a8cb-dcd0816db21e
📒 Files selected for processing (17)
gateway/gateway-runtime/policy-engine/internal/analytics/analytics.gogateway/gateway-runtime/policy-engine/internal/analytics/analytics_test.gogateway/gateway-runtime/policy-engine/internal/analytics/publishers/moesif.gogateway/gateway-runtime/policy-engine/internal/analytics/publishers/moesif_test.goplatform-api/internal/apperror/catalog.goplatform-api/internal/apperror/codes.goplatform-api/internal/repository/apikey.goplatform-api/internal/service/llm.goportals/ai-workspace/src/apis/providerTemplateApis.tsportals/ai-workspace/src/clients/choreoApiClient.tsportals/ai-workspace/src/clients/publicApiClient.tsportals/ai-workspace/src/pages/appShell/appShellPages/proxies/LLMProxyDeploymentsCard.tsxportals/ai-workspace/src/pages/appShell/appShellPages/proxies/LLMProxyOverviewTab.tsxportals/ai-workspace/src/pages/appShell/appShellPages/quickStart/lllmStepBanner/LLLMStepBanner.tsxportals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderConnectionTab.tsxportals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderDeploymentsCard.tsxportals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderOverviewTab.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@platform-api/internal/service/llm.go`:
- Around line 217-218: Update the Create validation around ValidateExternalURL
to run only when req.Metadata.EndpointUrl contains a non-empty, trimmed value,
allowing nil, empty, or whitespace-only endpoint URLs to proceed. Preserve the
existing invalid-URL error for supplied values and align the behavior with the
optional endpoint handling in Update.
🪄 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 Plus
Run ID: de73297b-306b-4522-9861-0fd36ff70b5b
📒 Files selected for processing (21)
platform-api/internal/apperror/catalog.goplatform-api/internal/apperror/codes.goplatform-api/internal/repository/apikey.goplatform-api/internal/service/llm.goplatform-api/internal/utils/common.goplatform-api/internal/utils/openapi_spec_fetcher_test.goportals/ai-workspace/src/apis/providerTemplateApis.tsportals/ai-workspace/src/clients/choreoApiClient.tsportals/ai-workspace/src/clients/publicApiClient.tsportals/ai-workspace/src/pages/appShell/appShellPages/proxies/LLMProxyDeploymentsCard.tsxportals/ai-workspace/src/pages/appShell/appShellPages/proxies/LLMProxyOverviewTab.tsxportals/ai-workspace/src/pages/appShell/appShellPages/quickStart/lllmStepBanner/LLLMStepBanner.tsxportals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/AddNewProvider/GuardrailsSection.tsxportals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ProvidersList.tsxportals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderConnectionTab.tsxportals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderDeploymentsCard.tsxportals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderNew.tsxportals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderOverview.tsxportals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderOverviewTab.tsxportals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProvidersSummaryCard.tsxportals/ai-workspace/src/utils/providerTemplateDisplay.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderOverviewTab.tsx
- portals/ai-workspace/src/pages/appShell/appShellPages/proxies/LLMProxyOverviewTab.tsx
- platform-api/internal/apperror/catalog.go
- portals/ai-workspace/src/pages/appShell/appShellPages/quickStart/lllmStepBanner/LLLMStepBanner.tsx
- platform-api/internal/repository/apikey.go
- platform-api/internal/apperror/codes.go
- portals/ai-workspace/src/clients/choreoApiClient.ts
- portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderConnectionTab.tsx
- portals/ai-workspace/src/clients/publicApiClient.ts
- portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/ServiceProviderDeploymentsCard.tsx
- portals/ai-workspace/src/apis/providerTemplateApis.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/EditServiceProvider.tsx (1)
45-65: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAllow a blank context to use the backend default.
A blank value is sent as
undefinedand the backend normalizes it to/, butCONTEXT_PATTERN.test('')disables the form and shows an error. This prevents resetting a context and can block description-only edits for providers with no returned context.Proposed fix
+ const trimmedContext = context.trim(); const contextTooLong = context.length > MAX_CONTEXT_LENGTH; const contextFormatInvalid = - !isReadOnlyProvider && !CONTEXT_PATTERN.test(context.trim()); + !isReadOnlyProvider && + trimmedContext !== '' && + !CONTEXT_PATTERN.test(trimmedContext); ... - context: context.trim() || undefined, + context: trimmedContext || undefined,Also applies to: 94-96, 254-260
🤖 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 `@portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/EditServiceProvider.tsx` around lines 45 - 65, Update the context validation in EditServiceProviderForm so a blank, trimmed context is accepted as valid and can be submitted as undefined for backend normalization. Preserve CONTEXT_PATTERN validation for non-blank contexts, including the existing read-only behavior, and ensure the related submit/disabled checks at the referenced flows allow description-only edits and context resets.
♻️ Duplicate comments (1)
platform-api/internal/service/llm.go (1)
359-363: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not validate an omitted optional endpoint URL.
req.Metadata != nildoes not meanEndpointUrlwas supplied. A nil or blank endpoint becomes""and fails with “URL is required,” blocking metadata-only updates. Validate only when the trimmed endpoint is non-empty.Proposed fix
if req.Metadata != nil { endpointURL := strings.TrimSpace(utils.ValueOrEmpty(req.Metadata.EndpointUrl)) - if err := utils.ValidateExternalURL(context.Background(), endpointURL); err != nil { - return nil, apperror.ValidationFailed.New("The metadata endpointUrl must be a valid, publicly reachable URL.") + if endpointURL != "" { + if err := utils.ValidateExternalURL(context.Background(), endpointURL); err != nil { + return nil, apperror.ValidationFailed.New("The metadata endpointUrl must be a valid, publicly reachable URL.") + } } }🤖 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 `@platform-api/internal/service/llm.go` around lines 359 - 363, Update the metadata validation block in the request handling flow to call utils.ValidateExternalURL only when the trimmed endpointURL is non-empty. Preserve validation and the existing error response for supplied URLs, while allowing nil or blank EndpointUrl values in metadata-only updates.
🤖 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 `@platform-api/internal/utils/common.go`:
- Around line 447-459: Update ValidateExternalURL and its outbound request path
to resolve hostnames and reject any private, loopback, link-local, metadata, or
otherwise non-public addresses, not just IP literals. Add dial-time IP
validation to prevent DNS rebinding, and apply the same validation to redirect
destinations using the existing URL validation flow and relevant HTTP
transport/client hooks.
---
Outside diff comments:
In
`@portals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/EditServiceProvider.tsx`:
- Around line 45-65: Update the context validation in EditServiceProviderForm so
a blank, trimmed context is accepted as valid and can be submitted as undefined
for backend normalization. Preserve CONTEXT_PATTERN validation for non-blank
contexts, including the existing read-only behavior, and ensure the related
submit/disabled checks at the referenced flows allow description-only edits and
context resets.
---
Duplicate comments:
In `@platform-api/internal/service/llm.go`:
- Around line 359-363: Update the metadata validation block in the request
handling flow to call utils.ValidateExternalURL only when the trimmed
endpointURL is non-empty. Preserve validation and the existing error response
for supplied URLs, while allowing nil or blank EndpointUrl values in
metadata-only updates.
🪄 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 Plus
Run ID: 1738a0ff-711b-471a-aaee-7e167c63df16
📒 Files selected for processing (3)
platform-api/internal/service/llm.goplatform-api/internal/utils/common.goportals/ai-workspace/src/pages/appShell/appShellPages/serviceProvider/EditServiceProvider.tsx
47a3a84
Purpose
This several UI bugs as follows.
Fix the listing issue in provider templates listing.
Fix the Endpoint URL validation issue under the connection tab of the LLM provider template.
Resolves: [Bug]: LLM Provider Templates UI Bugs #2891
Fix the issue of a duplicate API key name showing a generic error under LLM provider / LLM proxy
Resolves: #2880
resolves: #2895