chore: add assistant deployment REST APIs#145
Merged
Conversation
📝 WalkthroughWalkthroughThis PR adds REST CRUD endpoints for assistant deployments across five channels: Phone, API, Webplugin, WhatsApp, and Debugger. It introduces OpenAPI specs, generated Go types, structured error codes, Gin handlers for create/get/get-all operations, router registration, and test coverage. ChangesAssistant Deployment REST API — Create, Get, and Get-All Endpoints
Sequence Diagram(s)sequenceDiagram
participant Client
participant GinRouter
participant AssistantDeploymentApi
participant deploymentService
rect rgba(100, 149, 237, 0.5)
note over Client,deploymentService: Create Deployment Flow
Client->>GinRouter: POST /v1/assistant-deployment/create-{channel}-deployment
GinRouter->>AssistantDeploymentApi: Create{Channel}DeploymentRest(c)
AssistantDeploymentApi->>AssistantDeploymentApi: GetAuthPrinciple → 401/403
AssistantDeploymentApi->>AssistantDeploymentApi: BindJSON + validate fields → 400
AssistantDeploymentApi->>AssistantDeploymentApi: Convert audio/channel options to protos
AssistantDeploymentApi->>deploymentService: Create{Channel}Deployment(auth, assistantId, opts…)
deploymentService-->>AssistantDeploymentApi: deployment | error
AssistantDeploymentApi->>Client: 200 AssistantDeployment | 4xx/5xx ErrorResponse
end
rect rgba(144, 238, 144, 0.5)
note over Client,deploymentService: Get-All Flow
Client->>GinRouter: GET /v1/assistant-deployment/get-all-{channel}-deployment/{assistantId}?page=1&criterias=[…]
GinRouter->>AssistantDeploymentApi: GetAll{Channel}DeploymentRest(c)
AssistantDeploymentApi->>AssistantDeploymentApi: Auth + validate assistantId, page/pageSize
AssistantDeploymentApi->>AssistantDeploymentApi: Unmarshal criterias → assistant_api.Criteria[]
AssistantDeploymentApi->>deploymentService: GetAll{Channel}Deployment(auth, assistantId, criteria, paginate)
deploymentService-->>AssistantDeploymentApi: deployments[] + totalCount | error
AssistantDeploymentApi->>Client: 200 GetAllDeploymentResponse + Paginated
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/errors/create_assistant_deployment.go (1)
10-116:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAdd
pkg/errorstest updates for the new deployment error mappings.This backend change adds a large new exported error surface in
pkg/errors, but no corresponding*_test.goupdate is present in this package in the supplied cohort. Please add table-driven assertions for code/status/error-message mappings (at least one case per new deployment operation family).As per coding guidelines, backend changes in
api/**,pkg/**, orcmd/**must include corresponding*_test.goupdates in the same package.Also applies to: 173-664
🤖 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 `@pkg/errors/create_assistant_deployment.go` around lines 10 - 116, Create or update a test file (create_assistant_deployment_test.go) in the pkg/errors package to add table-driven test assertions for the new deployment error code mappings. Add test cases that verify the relationship between error codes (like CreateAssistantDebuggerDeploymentInvalidRequestCode, CreateAssistantPhoneDeploymentUnauthenticatedCode, CreateAssistantApiDeploymentCreateDeploymentCode, CreateAssistantWebpluginDeploymentInvalidIdealTimeoutCode, CreateAssistantWhatsappDeploymentMissingProviderCode, and corresponding GetAssistant* and GetAllAssistant* error codes) and their corresponding HTTP status codes and error messages. Include at least one test case per deployment operation family (Debugger, Phone, Api, Webplugin, Whatsapp) covering Create, Get, and GetAll operations to ensure all newly added error constants are properly mapped.Source: Coding guidelines
🧹 Nitpick comments (3)
api/assistant-api/api/assistant-deployment/get_assistant_deployment_rest_test.go (1)
11-15: ⚡ Quick winFix import grouping order to match the repository’s Go import policy.
The import block currently mixes external packages with
github.com/rapidaai/*. Split/import-order as: stdlib, external, thengithub.com/rapidaai/*.Suggested import block
import ( "encoding/json" "errors" "net/http" "net/http/httptest" "net/url" "testing" "github.com/gin-gonic/gin" - pkg_errors "github.com/rapidaai/pkg/errors" - "github.com/rapidaai/pkg/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + pkg_errors "github.com/rapidaai/pkg/errors" + "github.com/rapidaai/pkg/types" )As per coding guidelines, "Go import groups must follow order: stdlib, external, then github.com/rapidaai (enforced by goimports in golangci-lint)".
🤖 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 `@api/assistant-api/api/assistant-deployment/get_assistant_deployment_rest_test.go` around lines 11 - 15, The imports in the get_assistant_deployment_rest_test.go file are not grouped correctly according to the repository's Go import policy. Reorganize the import block at lines 11-15 to follow the correct order: first external packages like github.com/gin-gonic/gin and github.com/stretchr/testify packages, then add a blank line separator, followed by github.com/rapidaai packages (pkg_errors and types). This ensures compliance with the goimports and golangci-lint enforced convention of stdlib, external, then github.com/rapidaai imports.Source: Coding guidelines
api/assistant-api/api/assistant-deployment/get_assistant_api_deployment_rest.go (1)
1-1: ⚡ Quick winRename new API-layer files to the required dot-qualified naming convention.
The added files use underscore-based names, which violates the API file naming rule and makes convention-based discovery/tooling inconsistent.
api/assistant-api/api/assistant-deployment/get_assistant_api_deployment_rest.go#L1-L1: rename to a<domain>.<qualifier>.<entity>.gofilename.api/assistant-api/api/assistant-deployment/get_assistant_debugger_deployment_rest.go#L1-L1: rename to a<domain>.<qualifier>.<entity>.gofilename.api/assistant-api/api/assistant-deployment/get_assistant_phone_deployment_rest.go#L1-L1: rename to a<domain>.<qualifier>.<entity>.gofilename.api/assistant-api/api/assistant-deployment/get_assistant_webplugin_deployment_rest.go#L1-L1: rename to a<domain>.<qualifier>.<entity>.gofilename.api/assistant-api/api/assistant-deployment/get_assistant_whatsapp_deployment_rest.go#L1-L1: rename to a<domain>.<qualifier>.<entity>.gofilename.api/assistant-api/api/assistant-deployment/get_assistant_deployment_rest_test.go#L1-L1: align with the same convention while preserving_test.gosemantics.As per coding guidelines, "api/**/*.go: Use file naming convention ...go (e.g., knowledge.assistant.service.go) for Go files".
🤖 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 `@api/assistant-api/api/assistant-deployment/get_assistant_api_deployment_rest.go` at line 1, Multiple files in api/assistant-api/api/assistant-deployment/ are using underscore-based naming instead of the required dot-qualified convention. Rename each file to follow the <domain>.<qualifier>.<entity>.go pattern: in api/assistant-api/api/assistant-deployment/get_assistant_api_deployment_rest.go (anchor), rename to use dots instead of underscores following the pattern where domain is the top-level API layer, qualifier describes the deployment type, and entity is deployment; apply the same dot-qualified naming convention to api/assistant-api/api/assistant-deployment/get_assistant_debugger_deployment_rest.go (sibling), api/assistant-api/api/assistant-deployment/get_assistant_phone_deployment_rest.go (sibling), api/assistant-api/api/assistant-deployment/get_assistant_webplugin_deployment_rest.go (sibling), and api/assistant-api/api/assistant-deployment/get_assistant_whatsapp_deployment_rest.go (sibling); for api/assistant-api/api/assistant-deployment/get_assistant_deployment_rest_test.go (sibling), apply the same convention while preserving the _test.go suffix for Go test semantics.Source: Coding guidelines
api/assistant-api/api/assistant-deployment/create_assistant_phone_deployment_rest.go (1)
1-6: ⚡ Quick winRename this file to the repository API naming convention.
This filename uses underscore naming and diverges from the required API file naming scheme.
As per coding guidelines, "api/**/*.go: Use file naming convention ...go (e.g., knowledge.assistant.service.go) for Go files."🤖 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 `@api/assistant-api/api/assistant-deployment/create_assistant_phone_deployment_rest.go` around lines 1 - 6, Rename the file to follow the repository API naming convention of <domain>.<qualifier>.<entity>.go. The current filename create_assistant_phone_deployment_rest.go uses underscores and does not match the required naming scheme. Update the filename to use dots instead of underscores, organizing the components as domain, qualifier, and entity (for example, assistant.phone.deployment.go or deployment.phone.assistant.go depending on your domain structure). Ensure the new filename reflects the functional components of the file while adhering to the dot-separated convention.Source: Coding guidelines
🤖 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
`@api/assistant-api/api/assistant-deployment/create_assistant_debugger_deployment_rest_test.go`:
- Around line 263-299: The GetAssistantApiDeployment stub method in
createDebuggerDeploymentRestServiceStub is missing OutputAudio configuration
alongside the existing InputAudio setup. Add OutputAudio construction with
appropriate AudioType, AudioProvider, Id, Status, and AudioOptions (similar to
how InputAudio is built with "output" as AudioType), then add the constructed
OutputAudio to the deployment object being returned. This will automatically
improve test coverage for both GetAssistantApiDeployment and
GetAllAssistantApiDeployment (which delegates to this method) by ensuring
outputAudio mapping is properly asserted in both single and list GET operations.
- Around line 94-143: The CreatePhoneDeployment stub method is discarding
important fields that should be included in the returned entity to properly test
handler serialization. Remove idealTimeout, idealTimeoutBackoff,
idealTimeoutMessage, and outputAudio from the blank assignment statement on line
109. Then populate the returned phoneDeployment entity by: converting
outputAudio using the deploymentAudioProviderEntityFromProto helper (same
pattern as inputAudio) and setting it on the entity, and adding the
timeout-related fields (idealTimeout, idealTimeoutBackoff, idealTimeoutMessage)
to the AssistantDeploymentBehavior section of the entity to match the full shape
expected by handlers.
In
`@api/assistant-api/api/assistant-deployment/create_assistant_phone_deployment_rest.go`:
- Line 50: The error logging statements at both locations in
create_assistant_phone_deployment_rest.go are logging raw error text directly
using %v format, which risks exposing sensitive information or internal details.
At line 50 in the Errorf call within the invalid request error handling, and at
line 244 (referenced as "Also applies to"), replace the raw error logging with
stable, sanitized error codes and messages instead. Extract only non-sensitive
diagnostic information from the error and log that separately, avoiding direct
exposure of the underlying error details in the log output.
- Around line 8-19: The import block in
create_assistant_phone_deployment_rest.go needs to be reorganized into three
separate groups as required by goimports linting. Group the imports as follows:
first place the standard library imports (net/http and strconv), then add a
blank line and place the external third-party import (github.com/gin-gonic/gin),
then add another blank line and place all the github.com/rapidaai imports
together (openapi, pkg_errors, pkg/types, pkg/utils, pkg/validator, and
assistant_api). This three-group organization follows the Go import convention
and will satisfy the golangci-lint goimports checker.
In
`@api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go`:
- Around line 80-95: Add an upper-bound validation for the pageSize parameter to
prevent oversized list requests. In each of the five files
(api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go
lines 80-95,
api/assistant-api/api/assistant-deployment/get_all_assistant_debugger_deployment_rest.go
lines 80-95,
api/assistant-api/api/assistant-deployment/get_all_assistant_phone_deployment_rest.go
lines 80-95,
api/assistant-api/api/assistant-deployment/get_all_assistant_webplugin_deployment_rest.go
lines 80-95, and
api/assistant-api/api/assistant-deployment/get_all_assistant_whatsapp_deployment_rest.go
lines 80-95), extend the pageSize validation logic to reject values above a
shared maximum threshold (for example 100 or 500). After the existing check for
err or pageSize == 0, add an additional condition that checks if pageSize
exceeds the maximum allowed value and returns the same
GetAllAssistantApiDeploymentInvalidRequest error response if it does.
- Around line 1-7: The five new handler files in
api/assistant-api/api/assistant-deployment/ use underscore naming convention but
violate the repository's required naming convention for api files. Rename the
following files to follow the <domain>.<qualifier>.<entity>.go pattern: In
api/assistant-api/api/assistant-deployment/, rename
get_all_assistant_api_deployment_rest.go to assistant.deployment.api.go, rename
get_all_assistant_debugger_deployment_rest.go to
assistant.deployment.debugger.go, rename
get_all_assistant_phone_deployment_rest.go to assistant.deployment.phone.go,
rename get_all_assistant_webplugin_deployment_rest.go to
assistant.deployment.webplugin.go, and rename
get_all_assistant_whatsapp_deployment_rest.go to
assistant.deployment.whatsapp.go. This aligns with the coding guideline for api
files to use dot-qualified naming convention instead of underscore-separated
names.
- Around line 212-214: The conversion of totalItems (int64) to uint32 in the
variable assignment totalItem := uint32(totalItems) lacks bounds validation,
which allows values exceeding uint32.MaxValue to silently wrap and produce
incorrect pagination metadata. Add bounds checking before the conversion in all
five handler files:
api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go
at lines 212-214,
api/assistant-api/api/assistant-deployment/get_all_assistant_debugger_deployment_rest.go
at lines 212-214,
api/assistant-api/api/assistant-deployment/get_all_assistant_phone_deployment_rest.go
at lines 225-227,
api/assistant-api/api/assistant-deployment/get_all_assistant_webplugin_deployment_rest.go
at lines 214-216, and
api/assistant-api/api/assistant-deployment/get_all_assistant_whatsapp_deployment_rest.go
at lines 177-179. Validate that totalItems does not exceed uint32.MaxValue
before the conversion; either clamp the value or return an error if the bounds
are violated.
In
`@api/assistant-api/api/assistant-deployment/get_assistant_api_deployment_rest.go`:
- Around line 62-64: Remove raw error interpolation from the logger.Errorf calls
in all five deployment GET handlers to prevent potential leakage of sensitive
data (secrets/credentials/PII) into log sinks. In
api/assistant-api/api/assistant-deployment/get_assistant_api_deployment_rest.go
lines 62-64,
api/assistant-api/api/assistant-deployment/get_assistant_debugger_deployment_rest.go
lines 62-64,
api/assistant-api/api/assistant-deployment/get_assistant_phone_deployment_rest.go
lines 62-64,
api/assistant-api/api/assistant-deployment/get_assistant_webplugin_deployment_rest.go
lines 62-64, and
api/assistant-api/api/assistant-deployment/get_assistant_whatsapp_deployment_rest.go
lines 62-64, replace the deploymentApi.logger.Errorf call that currently
interpolates the raw err value with a stable internal error code or context
string instead (optionally including only debug-safe fields, never the raw error
text).
In `@openapi/artifacts/assistant-api.yaml`:
- Around line 472-485: The `page` and `pageSize` query parameters in the OpenAPI
specification allow values of 0 through their schema defaults, but the server
handlers reject `page == 0` and `pageSize == 0`. Add `minimum: 1` constraint to
the schema for both the `page` parameter (lines 472-479) and the `pageSize`
parameter (lines 480-485) in the anchor location. Apply the identical fix to the
same parameter blocks at all sibling locations: lines 534-547, lines 596-609,
lines 658-671, and lines 724-733. This ensures the OpenAPI schema accurately
reflects the server's validation rules and keeps generated clients aligned with
runtime behavior.
---
Outside diff comments:
In `@pkg/errors/create_assistant_deployment.go`:
- Around line 10-116: Create or update a test file
(create_assistant_deployment_test.go) in the pkg/errors package to add
table-driven test assertions for the new deployment error code mappings. Add
test cases that verify the relationship between error codes (like
CreateAssistantDebuggerDeploymentInvalidRequestCode,
CreateAssistantPhoneDeploymentUnauthenticatedCode,
CreateAssistantApiDeploymentCreateDeploymentCode,
CreateAssistantWebpluginDeploymentInvalidIdealTimeoutCode,
CreateAssistantWhatsappDeploymentMissingProviderCode, and corresponding
GetAssistant* and GetAllAssistant* error codes) and their corresponding HTTP
status codes and error messages. Include at least one test case per deployment
operation family (Debugger, Phone, Api, Webplugin, Whatsapp) covering Create,
Get, and GetAll operations to ensure all newly added error constants are
properly mapped.
---
Nitpick comments:
In
`@api/assistant-api/api/assistant-deployment/create_assistant_phone_deployment_rest.go`:
- Around line 1-6: Rename the file to follow the repository API naming
convention of <domain>.<qualifier>.<entity>.go. The current filename
create_assistant_phone_deployment_rest.go uses underscores and does not match
the required naming scheme. Update the filename to use dots instead of
underscores, organizing the components as domain, qualifier, and entity (for
example, assistant.phone.deployment.go or deployment.phone.assistant.go
depending on your domain structure). Ensure the new filename reflects the
functional components of the file while adhering to the dot-separated
convention.
In
`@api/assistant-api/api/assistant-deployment/get_assistant_api_deployment_rest.go`:
- Line 1: Multiple files in api/assistant-api/api/assistant-deployment/ are
using underscore-based naming instead of the required dot-qualified convention.
Rename each file to follow the <domain>.<qualifier>.<entity>.go pattern: in
api/assistant-api/api/assistant-deployment/get_assistant_api_deployment_rest.go
(anchor), rename to use dots instead of underscores following the pattern where
domain is the top-level API layer, qualifier describes the deployment type, and
entity is deployment; apply the same dot-qualified naming convention to
api/assistant-api/api/assistant-deployment/get_assistant_debugger_deployment_rest.go
(sibling),
api/assistant-api/api/assistant-deployment/get_assistant_phone_deployment_rest.go
(sibling),
api/assistant-api/api/assistant-deployment/get_assistant_webplugin_deployment_rest.go
(sibling), and
api/assistant-api/api/assistant-deployment/get_assistant_whatsapp_deployment_rest.go
(sibling); for
api/assistant-api/api/assistant-deployment/get_assistant_deployment_rest_test.go
(sibling), apply the same convention while preserving the _test.go suffix for Go
test semantics.
In
`@api/assistant-api/api/assistant-deployment/get_assistant_deployment_rest_test.go`:
- Around line 11-15: The imports in the get_assistant_deployment_rest_test.go
file are not grouped correctly according to the repository's Go import policy.
Reorganize the import block at lines 11-15 to follow the correct order: first
external packages like github.com/gin-gonic/gin and github.com/stretchr/testify
packages, then add a blank line separator, followed by github.com/rapidaai
packages (pkg_errors and types). This ensures compliance with the goimports and
golangci-lint enforced convention of stdlib, external, then github.com/rapidaai
imports.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 67cfd7bd-0180-4592-ac72-c35a6ce12151
📒 Files selected for processing (24)
api/assistant-api/api/assistant-deployment/create_assistant_api_deployment_rest.goapi/assistant-api/api/assistant-deployment/create_assistant_debugger_deployment_rest_test.goapi/assistant-api/api/assistant-deployment/create_assistant_phone_deployment_rest.goapi/assistant-api/api/assistant-deployment/create_assistant_phone_deployment_rest_test.goapi/assistant-api/api/assistant-deployment/create_assistant_webplugin_deployment_rest.goapi/assistant-api/api/assistant-deployment/create_assistant_whatsapp_deployment_rest.goapi/assistant-api/api/assistant-deployment/create_other_deployments_rest_test.goapi/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.goapi/assistant-api/api/assistant-deployment/get_all_assistant_debugger_deployment_rest.goapi/assistant-api/api/assistant-deployment/get_all_assistant_phone_deployment_rest.goapi/assistant-api/api/assistant-deployment/get_all_assistant_webplugin_deployment_rest.goapi/assistant-api/api/assistant-deployment/get_all_assistant_whatsapp_deployment_rest.goapi/assistant-api/api/assistant-deployment/get_assistant_api_deployment_rest.goapi/assistant-api/api/assistant-deployment/get_assistant_debugger_deployment_rest.goapi/assistant-api/api/assistant-deployment/get_assistant_deployment_rest_test.goapi/assistant-api/api/assistant-deployment/get_assistant_phone_deployment_rest.goapi/assistant-api/api/assistant-deployment/get_assistant_webplugin_deployment_rest.goapi/assistant-api/api/assistant-deployment/get_assistant_whatsapp_deployment_rest.goapi/assistant-api/router/assistant.goopenapi/artifacts/assistant-api.yamlopenapi/artifacts/common.yamlopenapi/assistant.gen.goopenapi/common.gen.gopkg/errors/create_assistant_deployment.go
Comment on lines
94
to
143
| func (s *createDebuggerDeploymentRestServiceStub) CreatePhoneDeployment( | ||
| context.Context, | ||
| types.SimplePrinciple, | ||
| uint64, | ||
| *string, | ||
| *string, | ||
| *uint64, | ||
| *uint64, | ||
| *string, | ||
| *uint64, | ||
| string, | ||
| *protos.DeploymentAudioProvider, | ||
| *protos.DeploymentAudioProvider, | ||
| []*protos.Metadata, | ||
| _ context.Context, | ||
| _ types.SimplePrinciple, | ||
| assistantId uint64, | ||
| greeting *string, | ||
| mistake *string, | ||
| idealTimeout *uint64, | ||
| idealTimeoutBackoff *uint64, | ||
| idealTimeoutMessage *string, | ||
| maxSessionDuration *uint64, | ||
| phoneProviderName string, | ||
| inputAudio *protos.DeploymentAudioProvider, | ||
| outputAudio *protos.DeploymentAudioProvider, | ||
| phoneOptions []*protos.Metadata, | ||
| ) (*internal_assistant_entity.AssistantPhoneDeployment, error) { | ||
| return nil, errors.New("not implemented") | ||
| _, _, _, _, _ = mistake, idealTimeout, idealTimeoutBackoff, idealTimeoutMessage, outputAudio | ||
| s.createCalled = true | ||
| s.assistantId = assistantId | ||
| s.greeting = greeting | ||
| s.inputAudio = inputAudio | ||
| s.outputAudio = outputAudio | ||
| s.maxSessionDuration = maxSessionDuration | ||
| s.phoneProviderName = phoneProviderName | ||
| s.phoneOptions = phoneOptions | ||
| if s.createErr != nil { | ||
| return nil, s.createErr | ||
| } | ||
|
|
||
| inputAudioEntity := deploymentAudioProviderEntityFromProto(inputAudio) | ||
| phoneDeployment := &internal_assistant_entity.AssistantPhoneDeployment{ | ||
| AssistantDeploymentBehavior: internal_assistant_entity.AssistantDeploymentBehavior{ | ||
| AssistantDeployment: internal_assistant_entity.AssistantDeployment{ | ||
| AssistantId: assistantId, | ||
| }, | ||
| Greeting: greeting, | ||
| MaxSessionDuration: maxSessionDuration, | ||
| }, | ||
| AssistantDeploymentTelephony: internal_assistant_entity.AssistantDeploymentTelephony{ | ||
| TelephonyProvider: phoneProviderName, | ||
| }, | ||
| InputAudio: inputAudioEntity, | ||
| } | ||
| for _, option := range phoneOptions { | ||
| optionEntity := &internal_assistant_entity.AssistantDeploymentTelephonyOption{} | ||
| optionEntity.Key = option.GetKey() | ||
| optionEntity.Value = option.GetValue() | ||
| phoneDeployment.TelephonyOption = append(phoneDeployment.TelephonyOption, optionEntity) | ||
| } | ||
| return phoneDeployment, nil | ||
| } |
There was a problem hiding this comment.
CreatePhoneDeployment stub drops fields that handlers serialize.
At Line 109 the method discards idealTimeout/outputAudio, and the returned entity (Lines 123-135) only sets InputAudio. This under-specifies the phone deployment shape and can let response-mapping bugs pass unnoticed.
Suggested fix
func (s *createDebuggerDeploymentRestServiceStub) CreatePhoneDeployment(
@@
) (*internal_assistant_entity.AssistantPhoneDeployment, error) {
- _, _, _, _, _ = mistake, idealTimeout, idealTimeoutBackoff, idealTimeoutMessage, outputAudio
+ _, _, _ = mistake, idealTimeoutBackoff, idealTimeoutMessage
@@
inputAudioEntity := deploymentAudioProviderEntityFromProto(inputAudio)
+ outputAudioEntity := deploymentAudioProviderEntityFromProto(outputAudio)
phoneDeployment := &internal_assistant_entity.AssistantPhoneDeployment{
AssistantDeploymentBehavior: internal_assistant_entity.AssistantDeploymentBehavior{
AssistantDeployment: internal_assistant_entity.AssistantDeployment{
AssistantId: assistantId,
},
Greeting: greeting,
+ IdleTimeout: idealTimeout,
MaxSessionDuration: maxSessionDuration,
},
AssistantDeploymentTelephony: internal_assistant_entity.AssistantDeploymentTelephony{
TelephonyProvider: phoneProviderName,
},
- InputAudio: inputAudioEntity,
+ InputAudio: inputAudioEntity,
+ OutputAudio: outputAudioEntity,
}🤖 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
`@api/assistant-api/api/assistant-deployment/create_assistant_debugger_deployment_rest_test.go`
around lines 94 - 143, The CreatePhoneDeployment stub method is discarding
important fields that should be included in the returned entity to properly test
handler serialization. Remove idealTimeout, idealTimeoutBackoff,
idealTimeoutMessage, and outputAudio from the blank assignment statement on line
109. Then populate the returned phoneDeployment entity by: converting
outputAudio using the deploymentAudioProviderEntityFromProto helper (same
pattern as inputAudio) and setting it on the entity, and adding the
timeout-related fields (idealTimeout, idealTimeoutBackoff, idealTimeoutMessage)
to the AssistantDeploymentBehavior section of the entity to match the full shape
expected by handlers.
Comment on lines
+263
to
299
| func (s *createDebuggerDeploymentRestServiceStub) GetAssistantApiDeployment( | ||
| _ context.Context, | ||
| _ types.SimplePrinciple, | ||
| assistantId uint64, | ||
| ) (*internal_assistant_entity.AssistantApiDeployment, error) { | ||
| s.getCalled = true | ||
| s.assistantId = assistantId | ||
| if s.getErr != nil { | ||
| return nil, s.getErr | ||
| } | ||
| if s.getNil { | ||
| return nil, nil | ||
| } | ||
| greeting := "Hello" | ||
| inputAudio := &internal_assistant_entity.AssistantDeploymentAudio{ | ||
| AudioType: "input", | ||
| AudioProvider: "twilio", | ||
| } | ||
| inputAudio.Id = 321 | ||
| inputAudio.Status = type_enums.RECORD_ACTIVE | ||
| inputAudioOption := &internal_assistant_entity.AssistantDeploymentAudioOption{} | ||
| inputAudioOption.Key = "codec" | ||
| inputAudioOption.Value = "mulaw" | ||
| inputAudio.AudioOptions = append(inputAudio.AudioOptions, inputAudioOption) | ||
| deployment := &internal_assistant_entity.AssistantApiDeployment{ | ||
| AssistantDeploymentBehavior: internal_assistant_entity.AssistantDeploymentBehavior{ | ||
| AssistantDeployment: internal_assistant_entity.AssistantDeployment{ | ||
| AssistantId: assistantId, | ||
| }, | ||
| Greeting: &greeting, | ||
| }, | ||
| InputAudio: inputAudio, | ||
| } | ||
| deployment.Id = 654 | ||
| deployment.Status = type_enums.RECORD_ACTIVE | ||
| return deployment, nil | ||
| } |
There was a problem hiding this comment.
API deployment getter stub misses OutputAudio, weakening GET and GET-all assertions.
GetAssistantApiDeployment builds only InputAudio (Lines 277-295). Since GetAllAssistantApiDeployment (Line 439) delegates to this getter, both single and list tests miss coverage of outputAudio mapping.
Suggested fix
func (s *createDebuggerDeploymentRestServiceStub) GetAssistantApiDeployment(
@@
inputAudio := &internal_assistant_entity.AssistantDeploymentAudio{
AudioType: "input",
AudioProvider: "twilio",
}
@@
+ outputAudio := &internal_assistant_entity.AssistantDeploymentAudio{
+ AudioType: "output",
+ AudioProvider: "twilio",
+ }
+ outputAudio.Id = 322
+ outputAudio.Status = type_enums.RECORD_ACTIVE
+
deployment := &internal_assistant_entity.AssistantApiDeployment{
@@
- InputAudio: inputAudio,
+ InputAudio: inputAudio,
+ OutputAudio: outputAudio,
}Also applies to: 424-444
🤖 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
`@api/assistant-api/api/assistant-deployment/create_assistant_debugger_deployment_rest_test.go`
around lines 263 - 299, The GetAssistantApiDeployment stub method in
createDebuggerDeploymentRestServiceStub is missing OutputAudio configuration
alongside the existing InputAudio setup. Add OutputAudio construction with
appropriate AudioType, AudioProvider, Id, Status, and AudioOptions (similar to
how InputAudio is built with "output" as AudioType), then add the constructed
OutputAudio to the deployment object being returned. This will automatically
improve test coverage for both GetAssistantApiDeployment and
GetAllAssistantApiDeployment (which delegates to this method) by ensuring
outputAudio mapping is properly asserted in both single and list GET operations.
Comment on lines
+8
to
+19
| import ( | ||
| "net/http" | ||
| "strconv" | ||
|
|
||
| "github.com/gin-gonic/gin" | ||
| "github.com/rapidaai/openapi" | ||
| pkg_errors "github.com/rapidaai/pkg/errors" | ||
| "github.com/rapidaai/pkg/types" | ||
| "github.com/rapidaai/pkg/utils" | ||
| "github.com/rapidaai/pkg/validator" | ||
| assistant_api "github.com/rapidaai/protos" | ||
| ) |
There was a problem hiding this comment.
Split imports into the required groups to satisfy lint/tooling.
The import block currently mixes external and github.com/rapidaai/... imports in one group. Separate them into: stdlib, external, then github.com/rapidaai.
As per coding guidelines, "Go import groups must follow order: stdlib, external, then github.com/rapidaai (enforced by goimports in golangci-lint)."
🤖 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
`@api/assistant-api/api/assistant-deployment/create_assistant_phone_deployment_rest.go`
around lines 8 - 19, The import block in
create_assistant_phone_deployment_rest.go needs to be reorganized into three
separate groups as required by goimports linting. Group the imports as follows:
first place the standard library imports (net/http and strconv), then add a
blank line and place the external third-party import (github.com/gin-gonic/gin),
then add another blank line and place all the github.com/rapidaai imports
together (openapi, pkg_errors, pkg/types, pkg/utils, pkg/validator, and
assistant_api). This three-group organization follows the Go import convention
and will satisfy the golangci-lint goimports checker.
Source: Coding guidelines
|
|
||
| var request openapi.CreateAssistantPhoneDeploymentRequest | ||
| if err := c.ShouldBindJSON(&request); err != nil { | ||
| deploymentApi.logger.Errorf("create assistant phone deployment invalid request: %v", err) |
There was a problem hiding this comment.
Avoid logging raw errors that may contain sensitive internals.
On Line 50 and Line 244, %v logs raw error text directly. If upstream errors include secrets or internal details, those get persisted in logs. Prefer logging stable error codes/messages and only sanitized diagnostics.
Also applies to: 244-244
🤖 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
`@api/assistant-api/api/assistant-deployment/create_assistant_phone_deployment_rest.go`
at line 50, The error logging statements at both locations in
create_assistant_phone_deployment_rest.go are logging raw error text directly
using %v format, which risks exposing sensitive information or internal details.
At line 50 in the Errorf call within the invalid request error handling, and at
line 244 (referenced as "Also applies to"), replace the raw error logging with
stable, sanitized error codes and messages instead. Extract only non-sensitive
diagnostic information from the error and log that separately, avoiding direct
exposure of the underlying error details in the log output.
Comment on lines
+1
to
+7
| // Copyright (c) 2023-2025 RapidaAI | ||
| // Author: Prashant Srivastav <prashant@rapida.ai> | ||
| // | ||
| // Licensed under GPL-2.0 with Rapida Additional Terms. | ||
| // See LICENSE.md or contact sales@rapida.ai for commercial usage. | ||
| package assistant_deployment_api | ||
|
|
There was a problem hiding this comment.
Rename new handler files to the required dot-qualified API filename convention.
The five new filenames use underscore style and do not match the repository naming rule for api/**/*.go.
api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go#L1-L7: rename to the<domain>.<qualifier>.<entity>.goconvention.api/assistant-api/api/assistant-deployment/get_all_assistant_debugger_deployment_rest.go#L1-L7: rename to the same convention.api/assistant-api/api/assistant-deployment/get_all_assistant_phone_deployment_rest.go#L1-L7: rename to the same convention.api/assistant-api/api/assistant-deployment/get_all_assistant_webplugin_deployment_rest.go#L1-L7: rename to the same convention.api/assistant-api/api/assistant-deployment/get_all_assistant_whatsapp_deployment_rest.go#L1-L7: rename to the same convention.
As per coding guidelines, “api/**/*.go: Use file naming convention <domain>.<qualifier>.<entity>.go (e.g., knowledge.assistant.service.go)”.
📍 Affects 5 files
api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go#L1-L7(this comment)api/assistant-api/api/assistant-deployment/get_all_assistant_debugger_deployment_rest.go#L1-L7api/assistant-api/api/assistant-deployment/get_all_assistant_phone_deployment_rest.go#L1-L7api/assistant-api/api/assistant-deployment/get_all_assistant_webplugin_deployment_rest.go#L1-L7api/assistant-api/api/assistant-deployment/get_all_assistant_whatsapp_deployment_rest.go#L1-L7
🤖 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
`@api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go`
around lines 1 - 7, The five new handler files in
api/assistant-api/api/assistant-deployment/ use underscore naming convention but
violate the repository's required naming convention for api files. Rename the
following files to follow the <domain>.<qualifier>.<entity>.go pattern: In
api/assistant-api/api/assistant-deployment/, rename
get_all_assistant_api_deployment_rest.go to assistant.deployment.api.go, rename
get_all_assistant_debugger_deployment_rest.go to
assistant.deployment.debugger.go, rename
get_all_assistant_phone_deployment_rest.go to assistant.deployment.phone.go,
rename get_all_assistant_webplugin_deployment_rest.go to
assistant.deployment.webplugin.go, and rename
get_all_assistant_whatsapp_deployment_rest.go to
assistant.deployment.whatsapp.go. This aligns with the coding guideline for api
files to use dot-qualified naming convention instead of underscore-separated
names.
Source: Coding guidelines
Comment on lines
+80
to
+95
| if c.Query("pageSize") != "" { | ||
| pageSize, err := strconv.ParseUint(c.Query("pageSize"), 10, 32) | ||
| if err != nil || pageSize == 0 { | ||
| c.JSON(pkg_errors.GetAllAssistantApiDeploymentInvalidRequest.HTTPStatusCode, openapi.ErrorResponse{ | ||
| Code: utils.Ptr(pkg_errors.GetAllAssistantApiDeploymentInvalidRequest.HTTPStatusCodeInt32()), | ||
| Success: utils.Ptr(false), | ||
| Error: &openapi.Error{ | ||
| ErrorCode: utils.Ptr(openapi.Uint64String(pkg_errors.GetAllAssistantApiDeploymentInvalidRequest.CodeString())), | ||
| ErrorMessage: utils.Ptr(pkg_errors.GetAllAssistantApiDeploymentInvalidRequest.Error), | ||
| HumanMessage: utils.Ptr(pkg_errors.GetAllAssistantApiDeploymentInvalidRequest.ErrorMessage), | ||
| }, | ||
| }) | ||
| return | ||
| } | ||
| paginate.PageSize = uint32(pageSize) | ||
| } |
There was a problem hiding this comment.
Enforce a maximum pageSize at the handler boundary.
All five handlers accept any positive pageSize up to uint32 max. That permits oversized list requests and can degrade service availability under authenticated abuse or accidental misuse.
api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go#L80-L95: rejectpageSizeabove a shared max (for example 100/500) with the existing invalid-request error.api/assistant-api/api/assistant-deployment/get_all_assistant_debugger_deployment_rest.go#L80-L95: apply the same upper-bound validation.api/assistant-api/api/assistant-deployment/get_all_assistant_phone_deployment_rest.go#L80-L95: apply the same upper-bound validation.api/assistant-api/api/assistant-deployment/get_all_assistant_webplugin_deployment_rest.go#L80-L95: apply the same upper-bound validation.api/assistant-api/api/assistant-deployment/get_all_assistant_whatsapp_deployment_rest.go#L80-L95: apply the same upper-bound validation.
📍 Affects 5 files
api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go#L80-L95(this comment)api/assistant-api/api/assistant-deployment/get_all_assistant_debugger_deployment_rest.go#L80-L95api/assistant-api/api/assistant-deployment/get_all_assistant_phone_deployment_rest.go#L80-L95api/assistant-api/api/assistant-deployment/get_all_assistant_webplugin_deployment_rest.go#L80-L95api/assistant-api/api/assistant-deployment/get_all_assistant_whatsapp_deployment_rest.go#L80-L95
🤖 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
`@api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go`
around lines 80 - 95, Add an upper-bound validation for the pageSize parameter
to prevent oversized list requests. In each of the five files
(api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go
lines 80-95,
api/assistant-api/api/assistant-deployment/get_all_assistant_debugger_deployment_rest.go
lines 80-95,
api/assistant-api/api/assistant-deployment/get_all_assistant_phone_deployment_rest.go
lines 80-95,
api/assistant-api/api/assistant-deployment/get_all_assistant_webplugin_deployment_rest.go
lines 80-95, and
api/assistant-api/api/assistant-deployment/get_all_assistant_whatsapp_deployment_rest.go
lines 80-95), extend the pageSize validation logic to reject values above a
shared maximum threshold (for example 100 or 500). After the existing check for
err or pageSize == 0, add an additional condition that checks if pageSize
exceeds the maximum allowed value and returns the same
GetAllAssistantApiDeploymentInvalidRequest error response if it does.
Comment on lines
+212
to
+214
| totalItem := uint32(totalItems) | ||
| currentPage := paginate.GetPage() | ||
| c.JSON(http.StatusOK, openapi.GetAllAssistantApiDeploymentResponse{ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, verify the files exist and examine the code at specified lines
for file in \
"api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go" \
"api/assistant-api/api/assistant-deployment/get_all_assistant_debugger_deployment_rest.go" \
"api/assistant-api/api/assistant-deployment/get_all_assistant_phone_deployment_rest.go" \
"api/assistant-api/api/assistant-deployment/get_all_assistant_webplugin_deployment_rest.go" \
"api/assistant-api/api/assistant-deployment/get_all_assistant_whatsapp_deployment_rest.go"
do
echo "=== $file ==="
if [ -f "$file" ]; then
echo "EXISTS"
wc -l "$file"
else
echo "NOT FOUND"
fi
doneRepository: rapidaai/voice-ai
Length of output: 1032
🏁 Script executed:
# Now examine the specific line ranges in each file
echo "=== get_all_assistant_api_deployment_rest.go L210-220 ==="
sed -n '210,220p' "api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go"
echo -e "\n=== get_all_assistant_debugger_deployment_rest.go L210-220 ==="
sed -n '210,220p' "api/assistant-api/api/assistant-deployment/get_all_assistant_debugger_deployment_rest.go"
echo -e "\n=== get_all_assistant_phone_deployment_rest.go L223-233 ==="
sed -n '223,233p' "api/assistant-api/api/assistant-deployment/get_all_assistant_phone_deployment_rest.go"
echo -e "\n=== get_all_assistant_webplugin_deployment_rest.go L212-222 ==="
sed -n '212,222p' "api/assistant-api/api/assistant-deployment/get_all_assistant_webplugin_deployment_rest.go"
echo -e "\n=== get_all_assistant_whatsapp_deployment_rest.go L175-185 ==="
sed -n '175,185p' "api/assistant-api/api/assistant-deployment/get_all_assistant_whatsapp_deployment_rest.go"Repository: rapidaai/voice-ai
Length of output: 2096
🏁 Script executed:
# Check broader context for each file - look at function signatures and where totalItems comes from
echo "=== get_all_assistant_api_deployment_rest.go full context ==="
sed -n '180,230p' "api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go"Repository: rapidaai/voice-ai
Length of output: 1713
🏁 Script executed:
# Check for imports and validation patterns in one of the files
echo "=== Checking imports in get_all_assistant_api_deployment_rest.go ==="
head -50 "api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go"Repository: rapidaai/voice-ai
Length of output: 2221
🏁 Script executed:
# Find where totalItems comes from - look at earlier lines in first file
echo "=== get_all_assistant_api_deployment_rest.go - earlier context to find totalItems source ==="
sed -n '1,212p' "api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go" | tail -100Repository: rapidaai/voice-ai
Length of output: 4139
🏁 Script executed:
# Also check if there's validation on totalItems before the conversion
echo "=== Check for any validation or bounds checking before line 212 ==="
grep -n "totalItem" "api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go"Repository: rapidaai/voice-ai
Length of output: 339
🏁 Script executed:
# Check the entire function to understand the flow
wc -l "api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go"
echo "---"
cat -n "api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go" | grep -A 5 -B 5 "totalItem"Repository: rapidaai/voice-ai
Length of output: 1441
🏁 Script executed:
# Look at the service call to understand what totalItems type is
echo "=== Looking for service call that returns totalItems ==="
sed -n '50,150p' "api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go"Repository: rapidaai/voice-ai
Length of output: 4712
🏁 Script executed:
# Find the service definition for GetAllAssistantApiDeployment
echo "=== Searching for deploymentService type and GetAllAssistantApiDeployment method ==="
find . -type f -name "*.go" | xargs grep -l "GetAllAssistantApiDeployment" | head -10Repository: rapidaai/voice-ai
Length of output: 804
🏁 Script executed:
# Search for the service interface/implementation
rg -t go "func.*GetAllAssistantApiDeployment" -A 3Repository: rapidaai/voice-ai
Length of output: 8376
🏁 Script executed:
# Look for the return type of GetAllAssistantApiDeployment
rg -t go "func.*GetAllAssistantApiDeployment.*\(.*int64" -B 2 -A 10Repository: rapidaai/voice-ai
Length of output: 3050
🏁 Script executed:
# Alternative: look at assistant-api structure to find deploymentService field
rg -t go "deploymentService" "api/assistant-api" -B 2 -A 2 | head -50Repository: rapidaai/voice-ai
Length of output: 5398
🏁 Script executed:
# Examine the service implementation to see if there's validation on the count
echo "=== assistant.deployment.impl.service.go full GetAllAssistantApiDeployment implementation ==="
sed -n '/func (eService assistantDeploymentService) GetAllAssistantApiDeployment/,/^func /p' "api/assistant-api/internal/services/assistant/assistant.deployment.impl.service.go" | head -100Repository: rapidaai/voice-ai
Length of output: 1720
🏁 Script executed:
# Check if there's any validation or guards on cnt before returning
rg -t go "cnt" "api/assistant-api/internal/services/assistant/assistant.deployment.impl.service.go" -B 2 -A 2Repository: rapidaai/voice-ai
Length of output: 2417
🏁 Script executed:
# Check what database/ORM call is used for count
rg -t go "Count\|count" "api/assistant-api/internal/services/assistant/assistant.deployment.impl.service.go" -B 2 -A 2Repository: rapidaai/voice-ai
Length of output: 43
🏁 Script executed:
# Look for the Paginate function to understand what it does with cnt
find . -type f -name "*.go" | xargs grep -l "func.*Paginate" | grep -E "(gorm|model)" | head -5Repository: rapidaai/voice-ai
Length of output: 128
🏁 Script executed:
# Check if there's validation on the returned count anywhere in the call chain
rg -t go "totalItems.*<\|totalItems.*>\|totalItems.*validate\|totalItems.*check" -B 2 -A 2Repository: rapidaai/voice-ai
Length of output: 43
🏁 Script executed:
# Verify all 5 affected files use the exact same pattern without validation
for file in \
"api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go" \
"api/assistant-api/api/assistant-deployment/get_all_assistant_debugger_deployment_rest.go" \
"api/assistant-api/api/assistant-deployment/get_all_assistant_phone_deployment_rest.go" \
"api/assistant-api/api/assistant-deployment/get_all_assistant_webplugin_deployment_rest.go" \
"api/assistant-api/api/assistant-deployment/get_all_assistant_whatsapp_deployment_rest.go"
do
echo "=== $file ==="
grep -n "totalItem.*uint32\|uint32.*totalItem" "$file"
doneRepository: rapidaai/voice-ai
Length of output: 724
🏁 Script executed:
# Check if there's any math import or bounds checking logic in any of these files
rg -t go "math\." "api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go" "api/assistant-api/api/assistant-deployment/get_all_assistant_debugger_deployment_rest.go" "api/assistant-api/api/assistant-deployment/get_all_assistant_phone_deployment_rest.go" "api/assistant-api/api/assistant-deployment/get_all_assistant_webplugin_deployment_rest.go" "api/assistant-api/api/assistant-deployment/get_all_assistant_whatsapp_deployment_rest.go"Repository: rapidaai/voice-ai
Length of output: 43
🏁 Script executed:
# Check the entire import section for all 5 files to see current imports
echo "=== API deployment imports ==="
head -20 "api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go"
echo -e "\n=== Debugger deployment imports ==="
head -20 "api/assistant-api/api/assistant-deployment/get_all_assistant_debugger_deployment_rest.go"
echo -e "\n=== Phone deployment imports ==="
head -20 "api/assistant-api/api/assistant-deployment/get_all_assistant_phone_deployment_rest.go"
echo -e "\n=== Webplugin deployment imports ==="
head -20 "api/assistant-api/api/assistant-deployment/get_all_assistant_webplugin_deployment_rest.go"
echo -e "\n=== Whatsapp deployment imports ==="
head -20 "api/assistant-api/api/assistant-deployment/get_all_assistant_whatsapp_deployment_rest.go"Repository: rapidaai/voice-ai
Length of output: 2988
🏁 Script executed:
# Check the gorm paginated.go to understand what values cnt could have
cat "pkg/models/gorm/paginated.go"Repository: rapidaai/voice-ai
Length of output: 1030
🏁 Script executed:
# Check if int64 can theoretically be negative from GORM count operations
# Look at any edge cases in the service that might set cnt to unexpected values
rg -t go "cnt\s*=" "api/assistant-api/internal/services/assistant/assistant.deployment.impl.service.go" -B 3 -A 3Repository: rapidaai/voice-ai
Length of output: 43
Add bounds validation before converting totalItems from int64 to uint32.
All 5 handlers perform unchecked narrowing conversion totalItem := uint32(totalItems). While GORM's Count() returns non-negative values, the absence of bounds checking means counts exceeding uint32.MaxValue (4,294,967,295) will silently wrap, returning incorrect pagination metadata to the API client.
api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go#L212api/assistant-api/api/assistant-deployment/get_all_assistant_debugger_deployment_rest.go#L212api/assistant-api/api/assistant-deployment/get_all_assistant_phone_deployment_rest.go#L225api/assistant-api/api/assistant-deployment/get_all_assistant_webplugin_deployment_rest.go#L214api/assistant-api/api/assistant-deployment/get_all_assistant_whatsapp_deployment_rest.go#L177
Clamp or validate the value within uint32 bounds before assignment.
Suggested fix pattern
+import "math"
...
- totalItem := uint32(totalItems)
+ var totalItem uint32
+ switch {
+ case totalItems <= 0:
+ totalItem = 0
+ case totalItems > int64(math.MaxUint32):
+ totalItem = math.MaxUint32
+ default:
+ totalItem = uint32(totalItems)
+ }📝 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.
Suggested change
| totalItem := uint32(totalItems) | |
| currentPage := paginate.GetPage() | |
| c.JSON(http.StatusOK, openapi.GetAllAssistantApiDeploymentResponse{ | |
| var totalItem uint32 | |
| switch { | |
| case totalItems <= 0: | |
| totalItem = 0 | |
| case totalItems > int64(math.MaxUint32): | |
| totalItem = math.MaxUint32 | |
| default: | |
| totalItem = uint32(totalItems) | |
| } | |
| currentPage := paginate.GetPage() | |
| c.JSON(http.StatusOK, openapi.GetAllAssistantApiDeploymentResponse{ |
📍 Affects 5 files
api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go#L212-L214(this comment)api/assistant-api/api/assistant-deployment/get_all_assistant_debugger_deployment_rest.go#L212-L214api/assistant-api/api/assistant-deployment/get_all_assistant_phone_deployment_rest.go#L225-L227api/assistant-api/api/assistant-deployment/get_all_assistant_webplugin_deployment_rest.go#L214-L216api/assistant-api/api/assistant-deployment/get_all_assistant_whatsapp_deployment_rest.go#L177-L179
🤖 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
`@api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go`
around lines 212 - 214, The conversion of totalItems (int64) to uint32 in the
variable assignment totalItem := uint32(totalItems) lacks bounds validation,
which allows values exceeding uint32.MaxValue to silently wrap and produce
incorrect pagination metadata. Add bounds checking before the conversion in all
five handler files:
api/assistant-api/api/assistant-deployment/get_all_assistant_api_deployment_rest.go
at lines 212-214,
api/assistant-api/api/assistant-deployment/get_all_assistant_debugger_deployment_rest.go
at lines 212-214,
api/assistant-api/api/assistant-deployment/get_all_assistant_phone_deployment_rest.go
at lines 225-227,
api/assistant-api/api/assistant-deployment/get_all_assistant_webplugin_deployment_rest.go
at lines 214-216, and
api/assistant-api/api/assistant-deployment/get_all_assistant_whatsapp_deployment_rest.go
at lines 177-179. Validate that totalItems does not exceed uint32.MaxValue
before the conversion; either clamp the value or return an error if the bounds
are violated.
Comment on lines
+62
to
+64
| if err != nil { | ||
| deploymentApi.logger.Errorf("unable to get assistant api deployment: %v", err) | ||
| c.JSON(pkg_errors.GetAssistantApiDeploymentGetDeployment.HTTPStatusCode, openapi.ErrorResponse{ |
There was a problem hiding this comment.
Avoid logging raw upstream error strings in deployment GET handlers.
These handlers sanitize HTTP responses, but they still interpolate raw err text into logs. If upstream errors include secrets/credentials/PII, this can leak sensitive data into log sinks.
api/assistant-api/api/assistant-deployment/get_assistant_api_deployment_rest.go#L62-L64: log a stable internal error code/context (and optional debug-safe fields), not rawerrtext.api/assistant-api/api/assistant-deployment/get_assistant_debugger_deployment_rest.go#L62-L64: apply the same sanitized logging pattern.api/assistant-api/api/assistant-deployment/get_assistant_phone_deployment_rest.go#L62-L64: apply the same sanitized logging pattern.api/assistant-api/api/assistant-deployment/get_assistant_webplugin_deployment_rest.go#L62-L64: apply the same sanitized logging pattern.api/assistant-api/api/assistant-deployment/get_assistant_whatsapp_deployment_rest.go#L62-L64: apply the same sanitized logging pattern.
📍 Affects 5 files
api/assistant-api/api/assistant-deployment/get_assistant_api_deployment_rest.go#L62-L64(this comment)api/assistant-api/api/assistant-deployment/get_assistant_debugger_deployment_rest.go#L62-L64api/assistant-api/api/assistant-deployment/get_assistant_phone_deployment_rest.go#L62-L64api/assistant-api/api/assistant-deployment/get_assistant_webplugin_deployment_rest.go#L62-L64api/assistant-api/api/assistant-deployment/get_assistant_whatsapp_deployment_rest.go#L62-L64
🤖 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
`@api/assistant-api/api/assistant-deployment/get_assistant_api_deployment_rest.go`
around lines 62 - 64, Remove raw error interpolation from the logger.Errorf
calls in all five deployment GET handlers to prevent potential leakage of
sensitive data (secrets/credentials/PII) into log sinks. In
api/assistant-api/api/assistant-deployment/get_assistant_api_deployment_rest.go
lines 62-64,
api/assistant-api/api/assistant-deployment/get_assistant_debugger_deployment_rest.go
lines 62-64,
api/assistant-api/api/assistant-deployment/get_assistant_phone_deployment_rest.go
lines 62-64,
api/assistant-api/api/assistant-deployment/get_assistant_webplugin_deployment_rest.go
lines 62-64, and
api/assistant-api/api/assistant-deployment/get_assistant_whatsapp_deployment_rest.go
lines 62-64, replace the deploymentApi.logger.Errorf call that currently
interpolates the raw err value with a stable internal error code or context
string instead (optionally including only debug-safe fields, never the raw error
text).
Comment on lines
+472
to
+485
| - name: page | ||
| in: query | ||
| required: false | ||
| schema: | ||
| type: integer | ||
| format: uint32 | ||
| default: 1 | ||
| - name: pageSize | ||
| in: query | ||
| required: false | ||
| schema: | ||
| type: integer | ||
| format: uint32 | ||
| default: 20 |
There was a problem hiding this comment.
Constrain pagination params to match server validation.
Line [476]/[483] (and the same page/pageSize blocks on other get-all endpoints) allow 0, but handlers reject page == 0 and pageSize == 0. Add minimum: 1 to keep generated clients aligned with runtime behavior.
Suggested schema adjustment
- name: page
in: query
required: false
schema:
type: integer
format: uint32
default: 1
+ minimum: 1
- name: pageSize
in: query
required: false
schema:
type: integer
format: uint32
default: 20
+ minimum: 1Also applies to: 534-547, 596-609, 658-671, 724-733
🤖 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 `@openapi/artifacts/assistant-api.yaml` around lines 472 - 485, The `page` and
`pageSize` query parameters in the OpenAPI specification allow values of 0
through their schema defaults, but the server handlers reject `page == 0` and
`pageSize == 0`. Add `minimum: 1` constraint to the schema for both the `page`
parameter (lines 472-479) and the `pageSize` parameter (lines 480-485) in the
anchor location. Apply the identical fix to the same parameter blocks at all
sibling locations: lines 534-547, lines 596-609, lines 658-671, and lines
724-733. This ensures the OpenAPI schema accurately reflects the server's
validation rules and keeps generated clients aligned with runtime behavior.
19 tasks
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.
Description
Type of Change
Related Issues
Fixes #
Checklist
General
Testing
Documentation
Security
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit