feat(api-service, dal): Add Agents API and DAL support#10679
Merged
Conversation
Introduce full Agents feature: adds AgentsModule and AgentsController with endpoints to create, list, get, update, and delete agents, plus endpoints to manage agent-integration links (add/list/update/remove). Implements DTOs, response mappers, and usecases for agent and agent-integration workflows. Adds DAL models, schemas, and repositories for Agent and AgentIntegration and exports them from libs/dal; wires repositories into the shared module. Also includes a minor provider.interface update (messageId optional field) used by some push providers.
✅ Deploy Preview for dashboard-v2-novu-staging canceled.
|
Contributor
|
Hey there and thank you for opening this pull request! 👋 We require pull request titles to follow specific formatting rules and it looks like your proposed title needs to be adjusted. Your PR title is: Requirements:
Expected format: Details: PR title must end with 'fixes TICKET-ID' (e.g., 'fixes NOV-123') or include ticket ID in branch name |
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 full agent management: DAL entities/repositories for agents and agent-integrations, nine use-cases, AgentsModule and controller with authenticated REST endpoints (cursor pagination, permissions), DTOs/mappers, shared module provider registration, e2e tests, and a small provider interface addition. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as AgentsController
participant UseCase as CreateAgent / AddAgentIntegration
participant Repo as AgentRepository / AgentIntegrationRepository
participant DB as MongoDB
Client->>API: POST /v1/agents {name, identifier}
API->>UseCase: createAgentCommand.execute(...)
UseCase->>Repo: findOne(identifier, env, org)
Repo->>DB: query
DB-->>Repo: no existing agent
UseCase->>Repo: create(agent)
Repo->>DB: insert
DB-->>Repo: created agent
Repo-->>UseCase: AgentEntity
UseCase-->>API: AgentResponseDto
API-->>Client: 201 Created
Client->>API: POST /v1/agents/:identifier/integrations {integrationIdentifier}
API->>UseCase: addAgentIntegrationCommand.execute(...)
UseCase->>Repo: find agent by identifier
Repo->>DB: query
DB-->>Repo: agent _id
UseCase->>Repo: find integration by identifier
Repo->>DB: query
DB-->>Repo: integration _id
UseCase->>Repo: find existing link
Repo->>DB: query
DB-->>Repo: none
UseCase->>Repo: create link
Repo->>DB: insert
DB-->>Repo: link
Repo-->>UseCase: AgentIntegrationEntity
UseCase-->>API: AgentIntegrationResponseDto
API-->>Client: 201 Created
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 7
🧹 Nitpick comments (17)
apps/api/src/app/agents/usecases/get-agent/get-agent.usecase.ts (1)
13-20: Consider narrowingselectto response fields only.Line 13-20 is correctly scoped, but using
'*'loads full documents. IftoAgentResponseneeds a subset, prefer arrayselectto reduce DB/network payload.As per coding guidelines: use array syntax as the default for
select, with'*'as explicit full-entity opt-in.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/usecases/get-agent/get-agent.usecase.ts` around lines 13 - 20, The findOne call on agentRepository is currently using '*' which returns the whole document; change it to an explicit array select containing only the fields required by toAgentResponse (e.g., the id/identifier, name, status, config, etc. used in ToAgentResponse) so you limit DB/network payload; update the select argument in the agentRepository.findOne(...) call to that array form rather than '*' and ensure toAgentResponse still receives all required fields.apps/api/src/app/agents/usecases/list-agents/list-agents.usecase.ts (1)
13-20: Prefer explicit field selection for list reads when possible.Line 13-20 is functionally correct, but list endpoints are good candidates for array
selectto avoid full-document reads if the mapper uses only a subset.As per coding guidelines: default to array syntax for
select, and use'*'only when full entity retrieval is needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/usecases/list-agents/list-agents.usecase.ts` around lines 13 - 20, The list use case currently calls this.agentRepository.find(..., '*', ...) which reads full documents; change the select argument to an explicit array of fields used by the mapper (e.g., ['_id','name','status','_environmentId','_organizationId','createdAt']) so list reads only necessary columns; update the this.agentRepository.find call in list-agents.usecase.ts to pass that array instead of '*' and ensure any mapped properties are included in the array.apps/api/src/app/agents/dtos/create-agent-request.dto.ts (1)
5-18: Add length and format guards forname/identifier.Current validation accepts any non-empty string. Consider adding sane max lengths and a URL-safe identifier pattern to prevent invalid or hard-to-route identifiers.
Proposed hardening
-import { IsNotEmpty, IsOptional, IsString } from 'class-validator'; +import { IsNotEmpty, IsOptional, IsString, Matches, MaxLength } from 'class-validator'; @@ `@ApiProperty`() `@IsString`() `@IsNotEmpty`() + `@MaxLength`(128) name: string; @@ `@ApiProperty`() `@IsString`() `@IsNotEmpty`() + `@MaxLength`(64) + `@Matches`(/^[a-z0-9_-]+$/) identifier: string; @@ `@ApiPropertyOptional`() `@IsString`() `@IsOptional`() + `@MaxLength`(1024) description?: string;As per coding guidelines: apps/api changes should include proper input validation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/dtos/create-agent-request.dto.ts` around lines 5 - 18, Add explicit length and format validation to the DTO properties: enforce a sensible max length (e.g., 100) on name and identifier (use `@MaxLength` on both, and optional `@MinLength` on name), and apply a URL-safe pattern for identifier using `@Matches` (e.g., allow only lowercase letters, numbers, hyphens/underscores) so identifiers are route-safe; also update the corresponding `@ApiProperty/`@ApiPropertyOptional metadata to reflect these constraints. Target the name and identifier fields in the CreateAgentRequest DTO (the properties named "name" and "identifier") and add the class-validator decorators and matching OpenAPI metadata descriptions accordingly.apps/api/src/app/agents/mappers/agent-response.mapper.ts (1)
5-6: Add a blank line before eachreturnstatement.This is a style-guideline mismatch only; behavior is fine.
As per coding guidelines:
**/*.{ts,tsx,js,jsx}: “Include a blank line before everyreturnstatement.”Also applies to: 18-19
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/mappers/agent-response.mapper.ts` around lines 5 - 6, Add a blank line immediately before every return statement in this module to satisfy the style guideline; specifically insert a blank line before the return inside the toAgentResponse function and before the other return(s) in this file (where other functions/methods produce AgentResponseDto or similar outputs) so every return has a preceding empty line.libs/dal/src/repositories/agent/agent.schema.ts (1)
32-33: Remove redundant index declarations.These indexes duplicate field-level
index: truedeclarations and add unnecessary index-management overhead.♻️ Proposed cleanup
-agentSchema.index({ _organizationId: 1 }); -agentSchema.index({ _environmentId: 1 }); agentSchema.index({ identifier: 1, _environmentId: 1 }, { unique: true });As per coding guidelines:
libs/dal/**: “Review with focus on data integrity and query performance.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/dal/src/repositories/agent/agent.schema.ts` around lines 32 - 33, Remove the redundant explicit index declarations agentSchema.index({ _organizationId: 1 }) and agentSchema.index({ _environmentId: 1 }) from agent.schema since those fields already declare index: true at the field level; keep the field-level index flags, delete the two agentSchema.index(...) lines, and quickly scan for any other duplicate index declarations to avoid index-management overhead.libs/dal/src/repositories/agent-integration/agent-integration.schema.ts (1)
41-42: Drop duplicate single-field indexes.These duplicate field-level
index: truedeclarations and are unnecessary.♻️ Proposed cleanup
-agentIntegrationSchema.index({ _agentId: 1 }); -agentIntegrationSchema.index({ _environmentId: 1 });As per coding guidelines:
libs/dal/**: “Review with focus on data integrity and query performance.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/dal/src/repositories/agent-integration/agent-integration.schema.ts` around lines 41 - 42, The file currently adds duplicate single-field indexes via agentIntegrationSchema.index({ _agentId: 1 }) and agentIntegrationSchema.index({ _environmentId: 1 }) while those fields already declare index: true; remove these explicit index() calls so each field-level index is created only once (delete the two agentIntegrationSchema.index(...) lines), and verify no required compound indexes rely on those lines before committing.apps/api/src/app/agents/usecases/add-agent-integration/add-agent-integration.usecase.ts (2)
30-37: Use array syntax forselectfor consistency.Line 36 uses string syntax
'_id'while other queries in this file use array syntax['_id']. As per coding guidelines, array syntax should be the default forselect.Suggested fix
const integration = await this.integrationRepository.findOne( { _id: command.integrationId, _environmentId: command.environmentId, _organizationId: command.organizationId, }, - '_id' + ['_id'] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/usecases/add-agent-integration/add-agent-integration.usecase.ts` around lines 30 - 37, The select argument in the integrationRepository.findOne call inside AddAgentIntegrationUseCase uses string syntax '_id' inconsistent with the rest of the file; update that call (the findOne invocation in add-agent-integration.usecase.ts) to use array syntax ['_id'] so it matches other queries and coding guidelines (leave the query filter object unchanged).
63-65: Add blank line beforereturnstatement.Per coding guidelines, include a blank line before every
returnstatement.Suggested fix
const link = await this.agentIntegrationRepository.create({ _agentId: agent._id, _integrationId: command.integrationId, _environmentId: command.environmentId, _organizationId: command.organizationId, }); + return toAgentIntegrationResponse(link); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/usecases/add-agent-integration/add-agent-integration.usecase.ts` around lines 63 - 65, Insert a blank line immediately before the final return in add-agent-integration.usecase.ts so the `return toAgentIntegrationResponse(link);` line is preceded by an empty line; locate the return inside the add-agent-integration use case (referencing `toAgentIntegrationResponse(link)` and the surrounding function/method in the AddAgentIntegration use case) and add the blank line to satisfy the coding guideline and linter formatting.apps/api/src/app/agents/usecases/update-agent-integration/update-agent-integration.usecase.ts (3)
46-48: Add blank line beforereturnstatement.Per coding guidelines, include a blank line before every
returnstatement.Suggested fix
if (existingLink._integrationId === command.integrationId) { + return toAgentIntegrationResponse(existingLink); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/usecases/update-agent-integration/update-agent-integration.usecase.ts` around lines 46 - 48, In update-agent-integration.usecase (the block comparing existingLink._integrationId to command.integrationId) add a blank line immediately before the return statement so the snippet reading "if (existingLink._integrationId === command.integrationId) { return toAgentIntegrationResponse(existingLink);" becomes separated by an empty line prior to "return" to satisfy the coding guideline; locate this conditional in the update-agent-integration.usecase function and insert the blank line before the return.
50-57: Use array syntax forselectfor consistency.Line 56 uses string syntax
'_id'while other queries use array syntax. Per coding guidelines, array syntax should be the default.Suggested fix
const integration = await this.integrationRepository.findOne( { _id: command.integrationId, _environmentId: command.environmentId, _organizationId: command.organizationId, }, - '_id' + ['_id'] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/usecases/update-agent-integration/update-agent-integration.usecase.ts` around lines 50 - 57, In update-agent-integration.usecase.ts, the integrationRepository.findOne call uses string select syntax '_id' which is inconsistent with the project's array select convention; change the select argument to use array syntax ['_id'] in the findOne invocation inside the UpdateAgentIntegrationUsecase (the const integration = await this.integrationRepository.findOne(...) call) so it matches other repository queries.
101-103: Add blank line beforereturnstatement.Per coding guidelines, include a blank line before every
returnstatement.Suggested fix
if (!updated) { throw new NotFoundException( `Agent-integration link "${command.agentIntegrationId}" was not found after update.` ); } + return toAgentIntegrationResponse(updated); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/usecases/update-agent-integration/update-agent-integration.usecase.ts` around lines 101 - 103, Add a blank line immediately before the final return statement "return toAgentIntegrationResponse(updated);" in the update-agent-integration.usecase (the function that updates the agent integration, e.g., the method wrapping this diff) so the return is separated by an empty line per coding guidelines; locate the return line and insert one empty line above it.apps/api/src/app/agents/agents.controller.ts (3)
54-59: Consider adding@SdkGroupNamedecorator.Per coding guidelines, SDK endpoints should be grouped with the
@SdkGroupNamedecorator using.as the subresource separator. Consider adding@SdkGroupName('Agents')at the class level.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/agents.controller.ts` around lines 54 - 59, Add the `@SdkGroupName` decorator at the class level to group SDK endpoints; insert `@SdkGroupName`('Agents') on the AgentsController class (near the existing decorators such as `@RequireAuthentication`(), `@ApiTags`('Agents') or `@Controller`('/agents')) so the controller (AgentsController) is correctly grouped using '.' as the subresource separator.
81-95: Add blank line beforereturnstatement.Per coding guidelines, include a blank line before every
returnstatement.Suggested fix
createAgent( `@UserSession`() user: UserSessionData, `@Body`() body: CreateAgentRequestDto ): Promise<AgentResponseDto> { + return this.createAgentUsecase.execute(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/agents.controller.ts` around lines 81 - 95, In the createAgent method, add a single blank line immediately before the return statement so the method body separates the setup (building CreateAgentCommand.create with userId/environmentId/organizationId/name/identifier/description) from the return; update the createAgent function to include that blank line above the existing return to satisfy the coding guideline.
141-163: List endpoint should support pagination.Similar to
listAgents, thelistAgentIntegrationsendpoint returns an unbounded array. Consider adding pagination support per coding guidelines.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/agents.controller.ts` around lines 141 - 163, The listAgentIntegrations endpoint returns an unbounded array; update it to support pagination by adding query parameters (e.g., page/limit or offset/limit) to the controller method signature, validate them via DTO or decorators, and pass them into ListAgentIntegrationsCommand (extend the command to accept pagination fields). Update the listAgentIntegrationsUsecase.execute call to forward pagination, change the response type from AgentIntegrationResponseDto[] to a paginated wrapper (e.g., PaginatedResponse<AgentIntegrationResponseDto>) and adjust ApiResponse/@ApiOperation metadata accordingly, and ensure the use case and repository layer honor limit/offset to return total/count and items.apps/api/src/app/agents/usecases/list-agent-integrations/list-agent-integrations.usecase.ts (1)
38-40: Add blank line beforereturnstatement.Per coding guidelines, include a blank line before every
returnstatement.Suggested fix
{ sort: { createdAt: -1 } } ); + return links.map(toAgentIntegrationResponse); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/usecases/list-agent-integrations/list-agent-integrations.usecase.ts` around lines 38 - 40, Insert a blank line immediately above the return statement that currently reads "return links.map(toAgentIntegrationResponse);" so there is an empty line separating preceding logic from the return; locate the return in the list-agent-integrations use case where toAgentIntegrationResponse is used and add the blank line to conform to the coding guideline.apps/api/src/app/agents/usecases/update-agent/update-agent.usecase.ts (2)
61-63: Add blank line beforereturnstatement.Per coding guidelines, include a blank line before every
returnstatement.Suggested fix
if (!updated) { throw new NotFoundException(`Agent with identifier "${command.identifier}" was not found.`); } + return toAgentResponse(updated); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/usecases/update-agent/update-agent.usecase.ts` around lines 61 - 63, Add a single blank line immediately before the return statement in the UpdateAgent use case function (the line returning toAgentResponse(updated) in update-agent.usecase.ts); locate the return inside the function (e.g., execute/handle method that produces `updated`) and insert one empty line above `return toAgentResponse(updated);` to satisfy the coding guideline.
17-24: Select only required fields for efficiency.The query retrieves all fields (
'*') but onlyexisting._idis used in subsequent operations.Suggested fix
const existing = await this.agentRepository.findOne( { identifier: command.identifier, _environmentId: command.environmentId, _organizationId: command.organizationId, }, - '*' + ['_id'] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/usecases/update-agent/update-agent.usecase.ts` around lines 17 - 24, The findOne call in update-agent.usecase.ts fetches all fields ('*') into existing but only existing._id is used later; change the query in AgentRepository.findOne (the call that sets the existing variable) to request just the _id field (e.g., ['_id']) to reduce data transfer and improve performance, keeping the same filter ({ identifier: command.identifier, _environmentId: command.environmentId, _organizationId: command.organizationId }) and leaving subsequent logic in UpdateAgentUsecase unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/app/agents/agents.controller.ts`:
- Around line 97-112: Update the listAgents endpoint to use pagination: add the
`@SdkUsePagination` decorator to the listAgents method, change its return type
from Promise<AgentResponseDto[]> to
Promise<PaginatedResponseDto<AgentResponseDto>>, and accept pagination query
params (limit, offset) from the request; pass those into
ListAgentsCommand.create (e.g., include limit and offset along with
environmentId and organizationId) so the ListAgentsUsecase returns a paginated
result consistent with other controllers like subscribersV1.controller.ts and
tenant.controller.ts.
In `@apps/api/src/app/agents/dtos/add-agent-integration-request.dto.ts`:
- Around line 2-10: Update the AddAgentIntegrationRequestDto so integrationId is
validated as a MongoDB ObjectId: replace the generic `@IsString`()/@IsNotEmpty()
usage on the integrationId property with the class-validator `@IsMongoId`()
decorator (and keep `@IsNotEmpty`() if you want to enforce presence) and add the
corresponding import for IsMongoId; ensure the property name integrationId in
class AddAgentIntegrationRequestDto is the same and that any tests or callers
supply a 24-hex-character ObjectId format.
In `@apps/api/src/app/agents/dtos/update-agent-integration-request.dto.ts`:
- Around line 2-10: The DTO UpdateAgentIntegrationRequestDto currently validates
integrationId only as a non-empty string; add MongoDB ObjectId validation by
importing and applying the `@IsMongoId`() decorator to integrationId so malformed
_id values are rejected early, keeping `@IsString`() and `@IsNotEmpty`() as desired
and updating imports accordingly.
In
`@apps/api/src/app/agents/usecases/add-agent-integration/add-agent-integration.command.ts`:
- Around line 10-12: The integrationId property on AddAgentIntegrationCommand is
only validated as a non-empty string and should be validated as a Mongo ObjectId
to prevent malformed IDs reaching the DAL; update the AddAgentIntegrationCommand
(the class containing the integrationId field) to use a MongoId validator (e.g.,
add the IsMongoId decorator from class-validator or an existing project-level
ObjectId validator) and include the necessary import so requests with invalid
ObjectId formats are rejected at validation time.
In `@apps/api/src/app/agents/usecases/create-agent/create-agent.usecase.ts`:
- Around line 13-34: Wrap the call to this.agentRepository.create in a try-catch
to handle Mongo duplicate-key errors: keep the existing pre-check using
this.agentRepository.findOne, but when calling create (the code that produces
`agent` and returns `toAgentResponse(agent)`), catch errors and if the error is
a MongoDB duplicate key (code 11000 or name 'MongoServerError' with code 11000),
throw a ConflictException with the same user-facing message used earlier (e.g.,
`An agent with identifier "${command.identifier}" already exists in this
environment.`); rethrow other errors unchanged. Ensure you reference the
create-agent.usecase.ts flow around this.agentRepository.create, the
ConflictException, and toAgentResponse.
In `@libs/dal/src/repositories/agent-integration/agent-integration.schema.ts`:
- Around line 8-27: The four ObjectId fields in the AgentIntegration schema
(_agentId, _integrationId, _organizationId, _environmentId) must be marked
required to enforce link-table integrity; update each field definition in
agent-integration.schema.ts (the objects for _agentId, _integrationId,
_organizationId, _environmentId) to include required: true while keeping their
type: Schema.Types.ObjectId, ref and index settings intact so malformed link
documents cannot be created.
In `@libs/dal/src/repositories/agent/agent.schema.ts`:
- Around line 18-27: The Agent schema's tenant keys _organizationId and
_environmentId are not marked required, weakening multi-tenant data integrity;
update the agent schema so both _organizationId and _environmentId include
required: true (keeping type: Schema.Types.ObjectId, ref: 'Organization' /
'Environment' and index: true) and, if appropriate, add a compound index on
{_organizationId, _environmentId} to enforce uniqueness/query performance for
the Agent model.
---
Nitpick comments:
In `@apps/api/src/app/agents/agents.controller.ts`:
- Around line 54-59: Add the `@SdkGroupName` decorator at the class level to group
SDK endpoints; insert `@SdkGroupName`('Agents') on the AgentsController class
(near the existing decorators such as `@RequireAuthentication`(),
`@ApiTags`('Agents') or `@Controller`('/agents')) so the controller
(AgentsController) is correctly grouped using '.' as the subresource separator.
- Around line 81-95: In the createAgent method, add a single blank line
immediately before the return statement so the method body separates the setup
(building CreateAgentCommand.create with
userId/environmentId/organizationId/name/identifier/description) from the
return; update the createAgent function to include that blank line above the
existing return to satisfy the coding guideline.
- Around line 141-163: The listAgentIntegrations endpoint returns an unbounded
array; update it to support pagination by adding query parameters (e.g.,
page/limit or offset/limit) to the controller method signature, validate them
via DTO or decorators, and pass them into ListAgentIntegrationsCommand (extend
the command to accept pagination fields). Update the
listAgentIntegrationsUsecase.execute call to forward pagination, change the
response type from AgentIntegrationResponseDto[] to a paginated wrapper (e.g.,
PaginatedResponse<AgentIntegrationResponseDto>) and adjust
ApiResponse/@ApiOperation metadata accordingly, and ensure the use case and
repository layer honor limit/offset to return total/count and items.
In `@apps/api/src/app/agents/dtos/create-agent-request.dto.ts`:
- Around line 5-18: Add explicit length and format validation to the DTO
properties: enforce a sensible max length (e.g., 100) on name and identifier
(use `@MaxLength` on both, and optional `@MinLength` on name), and apply a URL-safe
pattern for identifier using `@Matches` (e.g., allow only lowercase letters,
numbers, hyphens/underscores) so identifiers are route-safe; also update the
corresponding `@ApiProperty/`@ApiPropertyOptional metadata to reflect these
constraints. Target the name and identifier fields in the CreateAgentRequest DTO
(the properties named "name" and "identifier") and add the class-validator
decorators and matching OpenAPI metadata descriptions accordingly.
In `@apps/api/src/app/agents/mappers/agent-response.mapper.ts`:
- Around line 5-6: Add a blank line immediately before every return statement in
this module to satisfy the style guideline; specifically insert a blank line
before the return inside the toAgentResponse function and before the other
return(s) in this file (where other functions/methods produce AgentResponseDto
or similar outputs) so every return has a preceding empty line.
In
`@apps/api/src/app/agents/usecases/add-agent-integration/add-agent-integration.usecase.ts`:
- Around line 30-37: The select argument in the integrationRepository.findOne
call inside AddAgentIntegrationUseCase uses string syntax '_id' inconsistent
with the rest of the file; update that call (the findOne invocation in
add-agent-integration.usecase.ts) to use array syntax ['_id'] so it matches
other queries and coding guidelines (leave the query filter object unchanged).
- Around line 63-65: Insert a blank line immediately before the final return in
add-agent-integration.usecase.ts so the `return
toAgentIntegrationResponse(link);` line is preceded by an empty line; locate the
return inside the add-agent-integration use case (referencing
`toAgentIntegrationResponse(link)` and the surrounding function/method in the
AddAgentIntegration use case) and add the blank line to satisfy the coding
guideline and linter formatting.
In `@apps/api/src/app/agents/usecases/get-agent/get-agent.usecase.ts`:
- Around line 13-20: The findOne call on agentRepository is currently using '*'
which returns the whole document; change it to an explicit array select
containing only the fields required by toAgentResponse (e.g., the id/identifier,
name, status, config, etc. used in ToAgentResponse) so you limit DB/network
payload; update the select argument in the agentRepository.findOne(...) call to
that array form rather than '*' and ensure toAgentResponse still receives all
required fields.
In
`@apps/api/src/app/agents/usecases/list-agent-integrations/list-agent-integrations.usecase.ts`:
- Around line 38-40: Insert a blank line immediately above the return statement
that currently reads "return links.map(toAgentIntegrationResponse);" so there is
an empty line separating preceding logic from the return; locate the return in
the list-agent-integrations use case where toAgentIntegrationResponse is used
and add the blank line to conform to the coding guideline.
In `@apps/api/src/app/agents/usecases/list-agents/list-agents.usecase.ts`:
- Around line 13-20: The list use case currently calls
this.agentRepository.find(..., '*', ...) which reads full documents; change the
select argument to an explicit array of fields used by the mapper (e.g.,
['_id','name','status','_environmentId','_organizationId','createdAt']) so list
reads only necessary columns; update the this.agentRepository.find call in
list-agents.usecase.ts to pass that array instead of '*' and ensure any mapped
properties are included in the array.
In
`@apps/api/src/app/agents/usecases/update-agent-integration/update-agent-integration.usecase.ts`:
- Around line 46-48: In update-agent-integration.usecase (the block comparing
existingLink._integrationId to command.integrationId) add a blank line
immediately before the return statement so the snippet reading "if
(existingLink._integrationId === command.integrationId) { return
toAgentIntegrationResponse(existingLink);" becomes separated by an empty line
prior to "return" to satisfy the coding guideline; locate this conditional in
the update-agent-integration.usecase function and insert the blank line before
the return.
- Around line 50-57: In update-agent-integration.usecase.ts, the
integrationRepository.findOne call uses string select syntax '_id' which is
inconsistent with the project's array select convention; change the select
argument to use array syntax ['_id'] in the findOne invocation inside the
UpdateAgentIntegrationUsecase (the const integration = await
this.integrationRepository.findOne(...) call) so it matches other repository
queries.
- Around line 101-103: Add a blank line immediately before the final return
statement "return toAgentIntegrationResponse(updated);" in the
update-agent-integration.usecase (the function that updates the agent
integration, e.g., the method wrapping this diff) so the return is separated by
an empty line per coding guidelines; locate the return line and insert one empty
line above it.
In `@apps/api/src/app/agents/usecases/update-agent/update-agent.usecase.ts`:
- Around line 61-63: Add a single blank line immediately before the return
statement in the UpdateAgent use case function (the line returning
toAgentResponse(updated) in update-agent.usecase.ts); locate the return inside
the function (e.g., execute/handle method that produces `updated`) and insert
one empty line above `return toAgentResponse(updated);` to satisfy the coding
guideline.
- Around line 17-24: The findOne call in update-agent.usecase.ts fetches all
fields ('*') into existing but only existing._id is used later; change the query
in AgentRepository.findOne (the call that sets the existing variable) to request
just the _id field (e.g., ['_id']) to reduce data transfer and improve
performance, keeping the same filter ({ identifier: command.identifier,
_environmentId: command.environmentId, _organizationId: command.organizationId
}) and leaving subsequent logic in UpdateAgentUsecase unchanged.
In `@libs/dal/src/repositories/agent-integration/agent-integration.schema.ts`:
- Around line 41-42: The file currently adds duplicate single-field indexes via
agentIntegrationSchema.index({ _agentId: 1 }) and agentIntegrationSchema.index({
_environmentId: 1 }) while those fields already declare index: true; remove
these explicit index() calls so each field-level index is created only once
(delete the two agentIntegrationSchema.index(...) lines), and verify no required
compound indexes rely on those lines before committing.
In `@libs/dal/src/repositories/agent/agent.schema.ts`:
- Around line 32-33: Remove the redundant explicit index declarations
agentSchema.index({ _organizationId: 1 }) and agentSchema.index({
_environmentId: 1 }) from agent.schema since those fields already declare index:
true at the field level; keep the field-level index flags, delete the two
agentSchema.index(...) lines, and quickly scan for any other duplicate index
declarations to avoid index-management overhead.
🪄 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: 9fd74178-41c6-48ec-8f96-786d2ea93dee
📒 Files selected for processing (41)
apps/api/src/app.module.tsapps/api/src/app/agents/agents.controller.tsapps/api/src/app/agents/agents.module.tsapps/api/src/app/agents/dtos/add-agent-integration-request.dto.tsapps/api/src/app/agents/dtos/agent-integration-response.dto.tsapps/api/src/app/agents/dtos/agent-response.dto.tsapps/api/src/app/agents/dtos/create-agent-request.dto.tsapps/api/src/app/agents/dtos/index.tsapps/api/src/app/agents/dtos/update-agent-integration-request.dto.tsapps/api/src/app/agents/dtos/update-agent-request.dto.tsapps/api/src/app/agents/mappers/agent-response.mapper.tsapps/api/src/app/agents/usecases/add-agent-integration/add-agent-integration.command.tsapps/api/src/app/agents/usecases/add-agent-integration/add-agent-integration.usecase.tsapps/api/src/app/agents/usecases/create-agent/create-agent.command.tsapps/api/src/app/agents/usecases/create-agent/create-agent.usecase.tsapps/api/src/app/agents/usecases/delete-agent/delete-agent.command.tsapps/api/src/app/agents/usecases/delete-agent/delete-agent.usecase.tsapps/api/src/app/agents/usecases/get-agent/get-agent.command.tsapps/api/src/app/agents/usecases/get-agent/get-agent.usecase.tsapps/api/src/app/agents/usecases/index.tsapps/api/src/app/agents/usecases/list-agent-integrations/list-agent-integrations.command.tsapps/api/src/app/agents/usecases/list-agent-integrations/list-agent-integrations.usecase.tsapps/api/src/app/agents/usecases/list-agents/list-agents.command.tsapps/api/src/app/agents/usecases/list-agents/list-agents.usecase.tsapps/api/src/app/agents/usecases/remove-agent-integration/remove-agent-integration.command.tsapps/api/src/app/agents/usecases/remove-agent-integration/remove-agent-integration.usecase.tsapps/api/src/app/agents/usecases/update-agent-integration/update-agent-integration.command.tsapps/api/src/app/agents/usecases/update-agent-integration/update-agent-integration.usecase.tsapps/api/src/app/agents/usecases/update-agent/update-agent.command.tsapps/api/src/app/agents/usecases/update-agent/update-agent.usecase.tsapps/api/src/app/shared/shared.module.tslibs/dal/src/index.tslibs/dal/src/repositories/agent-integration/agent-integration.entity.tslibs/dal/src/repositories/agent-integration/agent-integration.repository.tslibs/dal/src/repositories/agent-integration/agent-integration.schema.tslibs/dal/src/repositories/agent-integration/index.tslibs/dal/src/repositories/agent/agent.entity.tslibs/dal/src/repositories/agent/agent.repository.tslibs/dal/src/repositories/agent/agent.schema.tslibs/dal/src/repositories/agent/index.tspackages/stateless/src/lib/provider/provider.interface.ts
Comment on lines
+97
to
+112
| @Get('/') | ||
| @ExternalApiAccessible() | ||
| @ApiResponse(AgentResponseDto, 200, true) | ||
| @ApiOperation({ | ||
| summary: 'List agents', | ||
| description: 'Returns all agents for the current environment.', | ||
| }) | ||
| @RequirePermissions(PermissionsEnum.WORKFLOW_READ) | ||
| listAgents(@UserSession() user: UserSessionData): Promise<AgentResponseDto[]> { | ||
| return this.listAgentsUsecase.execute( | ||
| ListAgentsCommand.create({ | ||
| environmentId: user.environmentId, | ||
| organizationId: user.organizationId, | ||
| }) | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how other list endpoints in the codebase implement pagination
rg -n "@SdkUsePagination" --type=ts -A5 apps/api/src/appRepository: novuhq/novu
Length of output: 1187
🏁 Script executed:
cat -n apps/api/src/app/agents/agents.controller.tsRepository: novuhq/novu
Length of output: 12929
Add @SdkUsePagination decorator and implement pagination for list endpoint.
Per coding guidelines, list endpoints must support pagination and use the @SdkUsePagination decorator. The listAgents endpoint currently returns an unbounded array instead of a paginated response. Other controllers in the codebase (e.g., subscribersV1.controller.ts, tenant.controller.ts) implement this pattern by adding the decorator and returning PaginatedResponseDto<AgentResponseDto> with pagination query parameters (e.g., limit, offset).
Update the endpoint to match the established pattern and prevent unbounded result sets.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/app/agents/agents.controller.ts` around lines 97 - 112, Update
the listAgents endpoint to use pagination: add the `@SdkUsePagination` decorator
to the listAgents method, change its return type from
Promise<AgentResponseDto[]> to Promise<PaginatedResponseDto<AgentResponseDto>>,
and accept pagination query params (limit, offset) from the request; pass those
into ListAgentsCommand.create (e.g., include limit and offset along with
environmentId and organizationId) so the ListAgentsUsecase returns a paginated
result consistent with other controllers like subscribersV1.controller.ts and
tenant.controller.ts.
Comment on lines
+2
to
+10
| import { IsNotEmpty, IsString } from 'class-validator'; | ||
|
|
||
| export class AddAgentIntegrationRequestDto { | ||
| @ApiProperty({ | ||
| description: 'The Novu integration document _id to link to this agent.', | ||
| }) | ||
| @IsString() | ||
| @IsNotEmpty() | ||
| integrationId: string; |
Contributor
There was a problem hiding this comment.
Tighten integrationId validation to Mongo _id format.
IsString + IsNotEmpty still accepts malformed IDs; this should be rejected at request validation time.
Suggested fix
-import { IsNotEmpty, IsString } from 'class-validator';
+import { IsMongoId, IsNotEmpty, IsString } from 'class-validator';
@@
`@IsString`()
`@IsNotEmpty`()
+ `@IsMongoId`()
integrationId: string;As per coding guidelines for apps/api/**: check for proper error handling and input validation.
📝 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
| import { IsNotEmpty, IsString } from 'class-validator'; | |
| export class AddAgentIntegrationRequestDto { | |
| @ApiProperty({ | |
| description: 'The Novu integration document _id to link to this agent.', | |
| }) | |
| @IsString() | |
| @IsNotEmpty() | |
| integrationId: string; | |
| import { IsMongoId, IsNotEmpty, IsString } from 'class-validator'; | |
| export class AddAgentIntegrationRequestDto { | |
| `@ApiProperty`({ | |
| description: 'The Novu integration document _id to link to this agent.', | |
| }) | |
| `@IsString`() | |
| `@IsNotEmpty`() | |
| `@IsMongoId`() | |
| integrationId: string; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/app/agents/dtos/add-agent-integration-request.dto.ts` around
lines 2 - 10, Update the AddAgentIntegrationRequestDto so integrationId is
validated as a MongoDB ObjectId: replace the generic `@IsString`()/@IsNotEmpty()
usage on the integrationId property with the class-validator `@IsMongoId`()
decorator (and keep `@IsNotEmpty`() if you want to enforce presence) and add the
corresponding import for IsMongoId; ensure the property name integrationId in
class AddAgentIntegrationRequestDto is the same and that any tests or callers
supply a 24-hex-character ObjectId format.
apps/api/src/app/agents/dtos/update-agent-integration-request.dto.ts
Outdated
Show resolved
Hide resolved
apps/api/src/app/agents/usecases/add-agent-integration/add-agent-integration.command.ts
Outdated
Show resolved
Hide resolved
Remove the ExternalApiAccessible import and decorator usages from agents controller endpoints to stop treating these routes as externally accessible. Add comprehensive agents e2e tests (apps/api/src/app/agents/e2e/agents.e2e.ts) covering CRUD, integrations linking, and cascade deletions. Add a new AgentsEarlyAccessDto (apps/api/src/app/support/dtos/agents-early-access.dto.ts) defining an `enabled` boolean for early access configuration.
commit: |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/app/agents/agents.controller.ts`:
- Around line 72-277: The controller's public agent handlers (createAgent,
listAgents, addAgentIntegration, listAgentIntegrations, updateAgentIntegration,
removeAgentIntegration, getAgent, updateAgent, deleteAgent) are missing the
ExternalApiAccessible decorator; add `@ExternalApiAccessible` above each of those
route handler methods so they are marked for user-API-key / official-SDK access,
preserving any existing decorators (e.g., `@Post`, `@Get`, `@RequirePermissions`) and
imports (ensure ExternalApiAccessible is imported where these methods are
defined).
- Around line 137-158: The controller's listAgentIntegrations method currently
returns an unbounded array; add pagination by applying the `@SdkUsePagination`
decorator, change the controller return type from
Promise<AgentIntegrationResponseDto[]> to the project's paginated response type
(e.g., Promise<PaginatedResponse<AgentIntegrationResponseDto>>), accept the
standard pagination input (the existing PaginationRequestDto or `@Query` params)
in the method signature, and propagate those pagination params into
ListAgentIntegrationsCommand (update the command to include page/limit/cursor as
appropriate) so listAgentIntegrationsUsecase.execute uses them to return a
paginated result; keep references to listAgentIntegrations,
listAgentIntegrationsUsecase, and ListAgentIntegrationsCommand when making these
changes.
🪄 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: 116d650f-74a4-4139-9597-1424f79eddc3
📒 Files selected for processing (3)
apps/api/src/app/agents/agents.controller.tsapps/api/src/app/agents/e2e/agents.e2e.tsapps/api/src/app/support/dtos/agents-early-access.dto.ts
Introduce cursor-based pagination for listing agents and agent integrations. Adds generic CursorPaginationQueryDto and specific query/response DTOs (list-agents-query/response, list-agent-integrations-query/response), updates controllers to accept pagination params and default sorting, and updates usecases to validate mutually exclusive cursors and return cursor-paginated responses. Implements repository methods listAgents and listAgentIntegrationsForAgent using findWithCursorBasedPagination, supports includeCursor, identifier/integrationId filters, and sort direction. Updates e2e tests to assert pagination fields and error handling when both before and after are provided.
Switch agent-integration API and internals to use the integration identifier (the external integration store identifier) rather than the internal MongoDB _id. Updated DTOs, controller params, commands, and usecases to accept integrationIdentifier; IntegrationRepository lookups now query by identifier and map to internal _id where needed. The mapper and response DTO now expose integrationIdentifier, error messages were updated, and list usecase resolves integration identifiers for returned links. E2E tests updated accordingly.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/api/src/app/agents/dtos/cursor-pagination-query.dto.ts (2)
34-39: Add@IsEnumvalidation fororderDirection.Without
@IsEnum(DirectionEnum)validation, invalid direction values will pass through the validation pipeline. TheDirectionEnumonly supportsASCandDESC.♻️ Proposed fix
+import { IsEnum, IsOptional, IsString } from 'class-validator'; -import { IsOptional, IsString } from 'class-validator'; `@ApiPropertyOptional`({ description: 'Direction of sorting', enum: DirectionEnum, }) + `@IsEnum`(DirectionEnum) `@IsOptional`() orderDirection?: DirectionEnum;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/dtos/cursor-pagination-query.dto.ts` around lines 34 - 39, Add enum validation for the orderDirection DTO property: update the property declaration for orderDirection to include the class-validator decorator `@IsEnum`(DirectionEnum) (alongside `@IsOptional`()) so only DirectionEnum values (ASC/DESC) pass validation; target the orderDirection property and the DirectionEnum symbol in cursor-pagination-query.dto.ts when applying the change.
7-23: Use@ApiPropertyOptionalfor optional properties.Per coding guidelines, optional TypeScript properties (
?) should use@ApiPropertyOptionalrather than@ApiPropertywithrequired: false.♻️ Proposed fix
- `@ApiProperty`({ + `@ApiPropertyOptional`({ description: 'Cursor for pagination indicating the starting point after which to fetch results.', type: String, - required: false, }) `@IsString`() `@IsOptional`() after?: string; - `@ApiProperty`({ + `@ApiPropertyOptional`({ description: 'Cursor for pagination indicating the ending point before which to fetch results.', type: String, - required: false, }) `@IsString`() `@IsOptional`() before?: string;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/dtos/cursor-pagination-query.dto.ts` around lines 7 - 23, Replace the `@ApiProperty`(...) decorators on the optional DTO fields after and before with `@ApiPropertyOptional` so the decorators match the TypeScript optional properties; locate the after and before properties in CursorPaginationQueryDto (or the DTO class in cursor-pagination-query.dto.ts) and swap the `@ApiProperty` usage (and remove required: false) to `@ApiPropertyOptional` for both fields.apps/api/src/app/agents/agents.controller.ts (1)
128-152: Add@SdkGroupNamedecorator for integration subresource endpoints.Per coding guidelines, SDK endpoints should be grouped with
@SdkGroupNamedecorator using.as the subresource separator. The agent-integration endpoints should useAgents.Integrationsgrouping.♻️ Proposed fix for addAgentIntegration
+import { SdkGroupName } from '@novu/application-generic'; `@Post`('/:identifier/integrations') + `@SdkGroupName`('Agents.Integrations') `@ApiResponse`(AgentIntegrationResponseDto, 201)Apply the same
@SdkGroupName('Agents.Integrations')decorator tolistAgentIntegrations,updateAgentIntegration, andremoveAgentIntegrationmethods.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/agents.controller.ts` around lines 128 - 152, Add the missing `@SdkGroupName`('Agents.Integrations') decorator to the integration subresource endpoints: annotate addAgentIntegration and the sibling methods listAgentIntegrations, updateAgentIntegration, and removeAgentIntegration with `@SdkGroupName`('Agents.Integrations') so they are grouped correctly in the SDK (use the exact decorator name and string 'Agents.Integrations' as the subresource separator).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/api/src/app/agents/agents.controller.ts`:
- Around line 128-152: Add the missing `@SdkGroupName`('Agents.Integrations')
decorator to the integration subresource endpoints: annotate addAgentIntegration
and the sibling methods listAgentIntegrations, updateAgentIntegration, and
removeAgentIntegration with `@SdkGroupName`('Agents.Integrations') so they are
grouped correctly in the SDK (use the exact decorator name and string
'Agents.Integrations' as the subresource separator).
In `@apps/api/src/app/agents/dtos/cursor-pagination-query.dto.ts`:
- Around line 34-39: Add enum validation for the orderDirection DTO property:
update the property declaration for orderDirection to include the
class-validator decorator `@IsEnum`(DirectionEnum) (alongside `@IsOptional`()) so
only DirectionEnum values (ASC/DESC) pass validation; target the orderDirection
property and the DirectionEnum symbol in cursor-pagination-query.dto.ts when
applying the change.
- Around line 7-23: Replace the `@ApiProperty`(...) decorators on the optional DTO
fields after and before with `@ApiPropertyOptional` so the decorators match the
TypeScript optional properties; locate the after and before properties in
CursorPaginationQueryDto (or the DTO class in cursor-pagination-query.dto.ts)
and swap the `@ApiProperty` usage (and remove required: false) to
`@ApiPropertyOptional` for both fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ca8cb5eb-2a56-48a5-b30a-376cf8783a65
📒 Files selected for processing (14)
apps/api/src/app/agents/agents.controller.tsapps/api/src/app/agents/dtos/cursor-pagination-query.dto.tsapps/api/src/app/agents/dtos/index.tsapps/api/src/app/agents/dtos/list-agent-integrations-query.dto.tsapps/api/src/app/agents/dtos/list-agent-integrations-response.dto.tsapps/api/src/app/agents/dtos/list-agents-query.dto.tsapps/api/src/app/agents/dtos/list-agents-response.dto.tsapps/api/src/app/agents/e2e/agents.e2e.tsapps/api/src/app/agents/usecases/list-agent-integrations/list-agent-integrations.command.tsapps/api/src/app/agents/usecases/list-agent-integrations/list-agent-integrations.usecase.tsapps/api/src/app/agents/usecases/list-agents/list-agents.command.tsapps/api/src/app/agents/usecases/list-agents/list-agents.usecase.tslibs/dal/src/repositories/agent-integration/agent-integration.repository.tslibs/dal/src/repositories/agent/agent.repository.ts
✅ Files skipped from review due to trivial changes (1)
- apps/api/src/app/agents/dtos/list-agent-integrations-query.dto.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/api/src/app/agents/usecases/list-agents/list-agents.usecase.ts
- apps/api/src/app/agents/usecases/list-agents/list-agents.command.ts
- apps/api/src/app/agents/dtos/index.ts
- apps/api/src/app/agents/usecases/list-agent-integrations/list-agent-integrations.usecase.ts
- libs/dal/src/repositories/agent/agent.repository.ts
- libs/dal/src/repositories/agent-integration/agent-integration.repository.ts
- apps/api/src/app/agents/e2e/agents.e2e.ts
Remove `index: true` from several fields in AgentIntegration and Agent schemas and drop the explicit _organizationId index on the Agent model. Indexes are now managed via explicit index definitions (retain _environmentId and the unique identifier+_environmentId compound index). This avoids duplicate/implicit index creation and centralizes index management.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
apps/api/src/app/agents/agents.controller.ts (2)
77-313:⚠️ Potential issue | 🟠 MajorRe-add
@ExternalApiAccessibleon the Agents API handlers.These routes expose the public Agents surface, but none of the handlers are marked as externally accessible anymore. That will keep them out of the user-API-key / official-SDK path even though the controller is implementing public resource endpoints.
As per coding guidelines:
Routes accessible via user API keys or the official SDK must use@ExternalApiAccessibledecorator in NestJS controllers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/agents.controller.ts` around lines 77 - 313, The controller methods that implement the public Agents surface (createAgent, listAgents, addAgentIntegration, listAgentIntegrations, updateAgentIntegration, removeAgentIntegration, getAgent, updateAgent, deleteAgent) are missing the `@ExternalApiAccessible` decorator; re-add `@ExternalApiAccessible` above each of those handler methods and ensure ExternalApiAccessible is imported where decorators are declared so these routes are exposed to user API keys / the official SDK.
100-126:⚠️ Potential issue | 🟡 MinorAdd
@SdkUsePaginationto both list endpoints.Both handlers already implement cursor-based pagination semantics, but the required SDK pagination decorator is still missing. That leaves the controller out of sync with the project’s list-endpoint contract.
As per coding guidelines:
List endpoints must support pagination and use@SdkUsePaginationdecorator.Also applies to: 154-185
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/agents.controller.ts` around lines 100 - 126, Add the missing `@SdkUsePagination` decorator to the controller's cursor-paginated list handlers: annotate the listAgents method (the function named listAgents) and the other list endpoint in the same controller with `@SdkUsePagination`, and add/import SdkUsePagination at the top of the file so the decorator is available; ensure the decorator is placed above the method decorators (e.g., above `@Get` and `@ApiOperation`) to align with the project's SDK pagination contract.
🧹 Nitpick comments (1)
apps/api/src/app/agents/agents.controller.ts (1)
128-152: Rename the integration subresource handlers to the standard controller verbs.
addAgentIntegrationandremoveAgentIntegrationbreak the controller naming convention used elsewhere.createAgentIntegration/deleteAgentIntegrationwould match the required pattern and keep the API surface consistent.As per coding guidelines:
Controller method names follow the pattern: getEntityName, listEntityName, createEntityName, updateEntityName, deleteEntityName.Also applies to: 215-242
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/agents/agents.controller.ts` around lines 128 - 152, Rename the nonstandard controller methods addAgentIntegration and removeAgentIntegration to follow the controller naming convention (createAgentIntegration and deleteAgentIntegration respectively); update the method names on the controller class, any corresponding decorator-declared handlers (e.g., `@Post`('/:identifier/integrations') and the delete route around lines referenced), and adjust any internal references (e.g., tests, route registration, or calls within this file) so the command creation still uses AddAgentIntegrationCommand.create and RemoveAgentIntegrationCommand.create as before but invoked from the newly named methods; ensure method signatures, decorators, permission decorators (RequirePermissions), and return types remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/api/src/app/agents/agents.controller.ts`:
- Around line 77-313: The controller methods that implement the public Agents
surface (createAgent, listAgents, addAgentIntegration, listAgentIntegrations,
updateAgentIntegration, removeAgentIntegration, getAgent, updateAgent,
deleteAgent) are missing the `@ExternalApiAccessible` decorator; re-add
`@ExternalApiAccessible` above each of those handler methods and ensure
ExternalApiAccessible is imported where decorators are declared so these routes
are exposed to user API keys / the official SDK.
- Around line 100-126: Add the missing `@SdkUsePagination` decorator to the
controller's cursor-paginated list handlers: annotate the listAgents method (the
function named listAgents) and the other list endpoint in the same controller
with `@SdkUsePagination`, and add/import SdkUsePagination at the top of the file
so the decorator is available; ensure the decorator is placed above the method
decorators (e.g., above `@Get` and `@ApiOperation`) to align with the project's SDK
pagination contract.
---
Nitpick comments:
In `@apps/api/src/app/agents/agents.controller.ts`:
- Around line 128-152: Rename the nonstandard controller methods
addAgentIntegration and removeAgentIntegration to follow the controller naming
convention (createAgentIntegration and deleteAgentIntegration respectively);
update the method names on the controller class, any corresponding
decorator-declared handlers (e.g., `@Post`('/:identifier/integrations') and the
delete route around lines referenced), and adjust any internal references (e.g.,
tests, route registration, or calls within this file) so the command creation
still uses AddAgentIntegrationCommand.create and
RemoveAgentIntegrationCommand.create as before but invoked from the newly named
methods; ensure method signatures, decorators, permission decorators
(RequirePermissions), and return types remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8e930894-ef9c-4b28-abde-5b95fcc76bf8
📒 Files selected for processing (13)
apps/api/src/app/agents/agents.controller.tsapps/api/src/app/agents/dtos/add-agent-integration-request.dto.tsapps/api/src/app/agents/dtos/agent-integration-response.dto.tsapps/api/src/app/agents/dtos/list-agent-integrations-query.dto.tsapps/api/src/app/agents/dtos/update-agent-integration-request.dto.tsapps/api/src/app/agents/e2e/agents.e2e.tsapps/api/src/app/agents/mappers/agent-response.mapper.tsapps/api/src/app/agents/usecases/add-agent-integration/add-agent-integration.command.tsapps/api/src/app/agents/usecases/add-agent-integration/add-agent-integration.usecase.tsapps/api/src/app/agents/usecases/list-agent-integrations/list-agent-integrations.command.tsapps/api/src/app/agents/usecases/list-agent-integrations/list-agent-integrations.usecase.tsapps/api/src/app/agents/usecases/update-agent-integration/update-agent-integration.command.tsapps/api/src/app/agents/usecases/update-agent-integration/update-agent-integration.usecase.ts
✅ Files skipped from review due to trivial changes (5)
- apps/api/src/app/agents/usecases/update-agent-integration/update-agent-integration.command.ts
- apps/api/src/app/agents/dtos/update-agent-integration-request.dto.ts
- apps/api/src/app/agents/dtos/agent-integration-response.dto.ts
- apps/api/src/app/agents/mappers/agent-response.mapper.ts
- apps/api/src/app/agents/e2e/agents.e2e.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/api/src/app/agents/dtos/list-agent-integrations-query.dto.ts
- apps/api/src/app/agents/dtos/add-agent-integration-request.dto.ts
- apps/api/src/app/agents/usecases/add-agent-integration/add-agent-integration.usecase.ts
- apps/api/src/app/agents/usecases/list-agent-integrations/list-agent-integrations.command.ts
- apps/api/src/app/agents/usecases/list-agent-integrations/list-agent-integrations.usecase.ts
…into feat-add-agent-entity
Replace the two-step findOne + delete flow in RemoveAgentIntegration with a single findOneAndDelete call to make removal atomic and avoid race conditions. Update BaseRepositoryV2.deleteMany to pass the session option only when provided (call deleteMany(query, { session }) if session exists, otherwise call deleteMany(query)) to ensure the correct Mongoose call signature and avoid unintended options being passed.
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Introduce AGENT_READ and AGENT_WRITE permission enums and add them to role permissions for OWNER, ADMIN, AUTHOR, and VIEWER. Update AgentsController endpoints to require AGENT_READ/AGENT_WRITE instead of using WORKFLOW_READ/WORKFLOW_WRITE, providing finer-grained access control for agent operations.
This was referenced Apr 13, 2026
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.
Introduce full Agents feature: adds AgentsModule and AgentsController with endpoints to create, list, get, update, and delete agents, plus endpoints to manage agent-integration links (add/list/update/remove). Implements DTOs, response mappers, and usecases for agent and agent-integration workflows. Adds DAL models, schemas, and repositories for Agent and AgentIntegration and exports them from libs/dal; wires repositories into the shared module. Also includes a minor provider.interface update (messageId optional field) used by some push providers.
What changed? Why was the change needed?
What changed
Adds a full Agents feature: REST APIs to create, list (cursor pagination), get, update, and delete agents plus endpoints to manage agent–integration links; supporting application-layer usecases, DTOs, response mappers; and DAL support (Agent and AgentIntegration schemas, entities, and repositories). Also makes push provider options accept an optional messageId for collapse-id behavior. The change enables programmatic agent management and reliable, paginated listing of agents and their linked integrations.
Affected areas
Key technical decisions
Testing
Added comprehensive e2e tests covering happy paths and negative cases (409, 404, 400) and verifying cascade deletion; no additional external dependencies introduced.
Screenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer